Skip to content

GCD, LCM: no endless rewriting of a lone integer; non-negative results - #2045

Merged
rocky merged 3 commits into
Mathics3:masterfrom
tombouctou:audit-gcd
Oct 18, 2026
Merged

rocky merged 3 commits into
Mathics3:masterfrom
tombouctou:audit-gcd

Conversation

@tombouctou

Copy link
Copy Markdown
Collaborator

GCD and LCM are Flat, and their rules used the pattern ns___Integer,
which also matches a single integer among other arguments. For GCD[1/2, 3],
GCD[2.5, 5] or LCM[1/2, 3] the evaluator kept rewriting GCD[3] to 3 and
back until $IterationLimit (in a session with a time limit: until the
limit).

The rules now take all the arguments, combine the integer ones (as
before: GCD[6, 9, x] is GCD[3, x]) and leave the expression alone when
there is nothing to combine.

GCD[-4] and LCM[-4] also gave -4; they are 4.

To reproduce on master:

GCD[1/2, 3]   (* $IterationLimit::itlim, or runs until a time limit *)
GCD[2.5, 5]   (* same *)
GCD[-4]       (* -4; should be 4 *)

Tests: new cases in test/builtin/intfns/test_divlike.py (test_divlike). They fail on current master and pass with this change.

The full pytest suite and the doctests pass with this change (run together with the other fixes from the same audit; the only pytest failures, in test_main.py and test_returncode.py, are the same as on master and come from the mathics script not being on PATH in our environment).

Found in a correctness audit of Mathics3 (differential checks against SymPy/mpmath/exact computation, and calls of every builtin with edge-case arguments).

🤖 Generated with Claude Code

GCD and LCM are Flat, and their rules used the pattern ns___Integer,
which also matches a single integer among other arguments. For GCD[1/2, 3],
GCD[2.5, 5] or LCM[1/2, 3] the evaluator kept rewriting GCD[3] to 3 and
back until $IterationLimit (in a session with a time limit: until the
limit).

The rules now take all the arguments, combine the integer ones (as
before: GCD[6, 9, x] is GCD[3, x]) and leave the expression alone when
there is nothing to combine.

GCD[-4] and LCM[-4] also gave -4; they are 4.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Comment thread test/builtin/intfns/test_divlike.py Outdated
("QuotientRemainder[5, 2.]", None, "{2, 1.}", None),
# GCD and LCM are Flat: a lone integer among other arguments must
# not be rewritten to itself until $IterationLimit.
("GCD[1/2, 3]", [], "GCD[1 / 2, 3]", None),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please do the magic to add meaningful assert failures.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 90a03f0: every new GCD/LCM case has a failure message saying what it checks.

Comment thread mathics/builtin/intfns/divlike.py Outdated

def combine_integer_arguments(builtin, ns, eval_fn) -> Optional[Expression]:
"""
Evaluate GCD or LCM: all integer arguments are combined into one,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this evaluates, but rather creates and returns an expression.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, it builds and returns the result rather than evaluating. Rewrote the docstring in 90a03f0 to say what comes back: an Integer when all arguments are integers, otherwise a new GCD[...]/LCM[...] with the integers combined, or None when there is nothing to combine.

tombouctou and others added 2 commits October 18, 2026 20:58
… failure messages for GCD/LCM tests

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@rocky

rocky commented Oct 18, 2026

Copy link
Copy Markdown
Member

LGTM

@rocky
rocky merged commit a14b7d6 into Mathics3:master Oct 18, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants