Macaulay2 / Macaulay2/M2

ARingRR and ARingRRR compare NaN equal to finite values

Open
#4,697 0 comments 0 reactions 0 assignees View on GitHub
Engine
Dominant language
Macaulay2
Stars
435
Forks
297
Avg merge
4d 20h
Merged PRs (30d)
11

Description

The real ARing comparison routines return zero when comparing NaN with 1, in either order. ARingRRR also reports `is_equal(NaN, 1)` and `is_zero(NaN)` as true. These results can silently classify an invalid arithmetic result as an ordinary finite value.

Reproducers in engine unit tests:

```cpp
M2::ARingRR R;
double nan = std::numeric_limits::quiet_NaN();
EXPECT_NE(R.compare_elems(nan, 1.0), 0); // fails
EXPECT_NE(R.compare_elems(1.0, nan), 0); // fails

M2::ARingRRR S(100);
M2::ARingRRR::Element a(S), one(S);
mpfr_set_nan(&a.value());
S.set(one, 1);
EXPECT_NE(S.compare_elems(a, one), 0); // fails
EXPECT_FALSE(S.is_equal(a, one)); // fails
EXPECT_FALSE(S.is_zero(a)); // fails
```

ARingRR compares the sign of a NaN difference. ARingRRR uses mpfr_cmp without handling its NaN case. A NaN ordering policy needs to be chosen; at minimum NaN should not compare equal to finite numbers. Reproduced on the Cornell arings branch at c156fdcba4 on AppleClang 21, arm64 macOS.

## Initialization also exposes the inconsistency

`ARingRR::init` stores `0.0`, whereas `ARingRRR::init` calls only `mpfr_init2`, leaving NaN at the requested precision. Because of the `is_zero(NaN)` defect above, this NaN is then indistinguishable from zero through that predicate. The initialization contract needs to be resolved: the regression below requests consistency with RR (an actual zero). Fixing only comparison does not satisfy that initialization expectation.

```cpp
M2::ARingRRR R(100);
M2::ARingRRR::ElementType a;
R.init(a);
EXPECT_FALSE(mpfr_nan_p(&a)); // fails: init leaves NaN
EXPECT_TRUE(mpfr_zero_p(&a)); // fails: the value is not zero
R.clear(a);
```

Regression blocks on the `retroactive` PR branch (MichaelABurr/M2#82):

- `TEST(ARingRR, DISABLED_compare_elems_nan)` in `M2/Macaulay2/e/unit-tests/ARingRRTest.cpp`.
- `TEST(ARingRRR, DISABLED_compare_elems_nan)` in `M2/Macaulay2/e/unit-tests/ARingRRRTest.cpp`.
- `TEST(ARingRRR, DISABLED_init_is_zero)` in `M2/Macaulay2/e/unit-tests/ARingRRRTest.cpp`, checking 53-, 100-, and 200-bit initialization against MPFR predicates independently of the faulty ARing comparisons.

From an engine CMake build directory:

```sh
./Macaulay2/e/M2-unit-tests --gtest_also_run_disabled_tests --gtest_filter='ARingRR.DISABLED_compare_elems_nan:ARingRRR.DISABLED_compare_elems_nan:ARingRRR.DISABLED_init_is_zero'
```

These are ordinary tests, not typed instantiations. Keep the initialization regression disabled until RRR initializes an actual zero, or an explicit contract decision supersedes that expectation; the NaN classification bug still needs to be addressed in either case.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with M2/Macaulay2/e/ARingRRTest.cpp and ARingRRRTest.cpp, then inspect the ARingRR and ARingRRR comparison and initialization routines. Run the supplied disabled-test filter from an engine CMake build directory. Done means NaN comparisons no longer report equality with finite values, while the initialization regression remains disabled until its contract is decided and implemented.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
backend, testing-qa
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
52/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.