Macaulay2 / Macaulay2/M2

Allow zero operands in ARingZZp subtract_multiple

Open
#4,699 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Engine
Dominant language
Macaulay2
Stars
435
Forks
297
Avg merge
4d 20h
Merged PRs (30d)
11

Description

`ARingZZp::subtract_multiple` documents that both factors must be nonzero. Unlike the FFPACK and FLINT implementations, it interprets a zero factor's encoded value as an exponent and produces an incorrect result if called outside that precondition.

The following reproducer illustrates the requested broader contract:

```cpp
M2::ARingZZp R(101);
M2::ARingZZp::Element a(R), zero(R), result(R);
R.set(a, 5);
R.set_zero(zero);
R.set(result, 7);
R.subtract_multiple(result, zero, a);
EXPECT_EQ(R.coerceToNonnegativeLongInteger(result), 7); // actual: 2
```

This is a request to extend the documented contract, not a failure for supported inputs. Add an enabled regression only if zero operands become supported and the implementation is updated accordingly. Reproduced on the Cornell arings branch at c156fdcba4 on AppleClang 21, arm64 macOS.

PR MichaelABurr/M2#82 removes `ARingZZp.DISABLED_subtractMultipleByZero` from `M2/Macaulay2/e/unit-tests/ARingZZpTest.cpp`: it used inputs outside the current documented contract, so it was a feature-request test rather than a defect regression. The request and reproducer remain here for a future contract decision.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start at ARingZZp::subtract_multiple and the disabled test in M2/Macaulay2/e/unit-tests/ARingZZpTest.cpp; compare the FFPACK and FLINT implementations mentioned in the issue. First resolve whether zero operands should be supported, then update the implementation and documented contract if so. Enable a regression using the supplied reproducer only after the new behavior is supported.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
backend
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.