Skip to content

A rational factor compiled by the published compile entry is a division - #481

Closed
enumeratio wants to merge 2 commits into
cortex-js:mainfrom
enumeratio:compile-rational-literal
Closed

enumeratio wants to merge 2 commits into
cortex-js:mainfrom
enumeratio:compile-rational-literal

Conversation

@enumeratio

Copy link
Copy Markdown
Contributor

Fix #480.

asRational recognized an exact rational by instanceof ExactNumericValue. The standalone compile bundle carries its own copy of that class, so for an expression boxed by compute-engine the test failed and the division lowering for Rational(p, q) factors did not apply: n(n-1)(n-2)/6 compiled to 0.16666666666666666 * … (559.9999999999999 at n = 16). asRational now also accepts a value by shape (a numeric radical and an array rational), so both copies agree. Every target that lowers through exactRationalDivisor benefits, including Python and the shader targets.

The new test loads JavaScriptTarget from a second module registry (jest.isolateModules) and compiles an expression boxed by the first, standing in for the two bundles:

  • n(n-1)(n-2)/6 with n declared an integer: a trailing / 6, exact for several n including 16.
  • n/49: _.n / 49, exact at every multiple of 49.
  • 2x/3 is _.x / 3 * 2 and x/3 is within an ulp of the engine's .N().

The three exactness tests fail without the change.

@arnog

arnog commented Oct 26, 2026

Copy link
Copy Markdown
Member

Thank you for the clear diagnosis and the test. Commit 1531783 on main fixes the cause: compile, core and the main entry are now built in one code-splitting run and share one copy of the engine, so every instanceof check in the compiler works across entries, not only the one in asRational. With the current build, your reproduction compiles to (_.n * (_.n + -2) * (_.n + -1)) / 6 and gives 560 at n = 16. test/consumer/require-smoke.cjs fails the build if compile.js stops sharing the engine chunk. The fix ships in the next release, and the changelog credits your report.

@arnog arnog closed this Oct 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Compiled n * (n - 1) * (n - 2) / 6 from the published compile entry multiplies by a rounded 1/6

2 participants