Skip to content

Compile arcsine to the arcsine, not to its conjugate - #696

Merged
Happypig375 merged 1 commit into
ASC-Community:masterfrom
Rafael-SOWNet:fix/compiled-arcsin-branch
Aug 4, 2026
Merged

Happypig375 merged 1 commit into
ASC-Community:masterfrom
Rafael-SOWNet:fix/compiled-arcsin-branch

Conversation

@Rafael-SOWNet

Copy link
Copy Markdown
Member

The defect

The FE compiler conjugated arcsine unconditionally (Functions/Compilation/IntoFE/FastExpression.cs, since October 2020):

case InstructionType.CALL_ARCSIN:
    stack.Push(Complex.Conjugate(Complex.Asin(stack.Pop())));

That is correct on the two branch cuts — the real arguments outside [-1, 1], where the library does take the lower side, and where the only test of it looks (CompilationFETest evaluates everything at x = 3). It is wrong everywhere else. Compiled arcsine was the conjugate of the arcsine over the whole rest of the plane:

sin(arcsin(0.5 + 0.1i)) arcsin(z) + arccos(z) at 0.5 + 0.1i
before 0.5 − 0.1i π/2 − 0.2299i
after 0.5 + 0.1i π/2

Measuring it rather than arguing about branches

Against Evaled — the library's own answer, and the thing a compiled expression is supposed to reproduce — over a grid of 63 points spanning both cuts, both sides of each (im = ±1e-9), and the interval between them:

before after
FE compiler, arcsin 15 / 63 63 / 63
Linq compiler, arcsin 59 / 63 63 / 63
FE and Linq, arccsc 60 / 62 62 / 62

Neither compiler was right. The FE one conjugated everywhere; the Linq one conjugated nowhere, so it missed exactly the four points on the cut, where it takes the opposite side from the rest of the library. They were wrong in complementary places, and had been for five years.

arccos, arctan, arcsec and arccot already agreed at every point and are untouched.

The fix

One helper, Core/Compilation/ComplexBranches.cs, that conjugates on the cut and nowhere else, used by both compilers and by arccsc (which is arcsine of the reciprocal, so it lands on the same cuts — for it, at the real arguments strictly inside [-1, 1]).

MathAllMethods.cs is generated, so MathAllMethods.tt carries the same change and the two are consistent.

The library's side of the cut is deliberately kept. arcsin(3) stays π/2 − 1.7627i here, where System.Numerics.Complex.Asin gives π/2 + 1.7627i. Which side of a cut to take is a convention, not a bug, and this PR does not touch it — pinned by TheLibrarysSideOfTheCutIsUnchanged. That is why nothing on the real axis changes and no existing test needed editing.

Why it mattered beyond compilation

Newton's method was the visible casualty. Started at a root of arcsin(x) - x*pi/3, it walked away from it: the derivative a conjugated arcsine implies has the wrong sign in the imaginary direction, which turns the root into a repeller. Traced, the imaginary part grew by a factor of about 21 per step from 4.5e-31 until the iteration left for a non-root near −0.0014 + 3.7998i. −1/2 is now found.

This is part of #115, not the whole of it. At the default StepCount of (10, 10) the grid has spacing 2 over [-10, 10], so it steps over both outlying roots regardless of this fix, and Solve still answers {0}. Raising the step count reaches them — which needs #695 to work at all.

Tests

Sources/Tests/UnitTests/Common/CompiledArcsinBranchTest.cs, 138 cases. 61 of them fail without the source change (verified by reverting it in place and re-running).

  • both compilers against Evaled across the grid, for arcsin and arccsc
  • sin(arcsin(z)) == z — the defining property, which the old code did not have
  • arcsin(z) + arccos(z) == π/2 off the real axis, which the existing CompilationFETest case for that pair never checked
  • the cut convention, unchanged

Full suite: Failed: 0, Passed: 4147, Skipped: 14, Total: 4161.

Noted, not fixed

arccot(0) compiles to NaN (Atan(1/0)) where Evaled gives π/2. Same family, different cause, and out of scope here.

🤖 Generated with Claude Code

The FE compiler conjugated arcsine unconditionally:

    stack.Push(Complex.Conjugate(Complex.Asin(stack.Pop())));

That is right on the two branch cuts -- the real arguments outside [-1, 1],
where the library does take the lower side and where the only test of it
looked -- and wrong everywhere else. Compiled arcsine was the conjugate of
the arcsine over the whole rest of the plane, so sin(arcsin(0.5 + 0.1i))
came back as 0.5 - 0.1i, and arcsin(z) + arccos(z) was not pi/2 off the
real axis.

Measured against Evaled over a grid of 63 points spanning both cuts, both
sides of each and the interval between them, the FE compiler agreed at 15.
The Linq compiler, which does not conjugate, agreed at 59: it misses the
four points on the cut, taking the other side of it from the rest of the
library. Neither was right; they were wrong in complementary places.

Both now go through one helper that conjugates on the cut and nowhere else,
which agrees with Evaled at every one of the 63 -- and at 62 of 62 for
arccosecant, which is arcsine of the reciprocal and so lands on the same
cuts. Nothing changes on the real axis, where every existing test looks.

Newton's method was the visible casualty. Started at a root of
arcsin(x) - x*pi/3 it walked away from it, because the derivative the
conjugated arcsine implies has the wrong sign in the imaginary direction,
which turns the root into a repeller: the imaginary part grew by a factor
of about 21 per step until the iteration left for elsewhere. -1/2 is now
found. This is part of #115
rather than the whole of it -- at the default step count the grid steps
over both of the outlying roots regardless.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@codecov-commenter

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.45%. Comparing base (90c00a8) to head (c1ba084).
⚠️ Report is 58 commits behind head on master.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #696      +/-   ##
==========================================
- Coverage   80.99%   80.45%   -0.54%     
==========================================
  Files         155      157       +2     
  Lines       13687    12921     -766     
  Branches     1957     2123     +166     
==========================================
- Hits        11086    10396     -690     
+ Misses       1990     1917      -73     
+ Partials      611      608       -3     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Rafael-SOWNet

Copy link
Copy Markdown
Member Author

CI, run on my fork across the full matrix, since the run here is held at action_required:

https://github.com/Rafael-SOWNet/AngouriMath/actions/runs/30921443094

job result tests
Test (windows-latest) success Failed: 0, Passed: 4147, Skipped: 14, Total: 4161
Test (ubuntu-latest) success Failed: 0, Passed: 4147, Skipped: 14, Total: 4161
Test (macos-latest) success Failed: 0, Passed: 4147, Skipped: 14, Total: 4161

That matters more than usual here, because the branch-cut convention this PR preserves depends on how System.Numerics.Complex.Asin behaves, and all three platforms agree.

The branch it ran on, ci-check/arcsin-branch, is this PR's head plus one commit that only touches .github/workflows/CSharpTest.yml, so the run can be triggered at all. Everything under Sources/ is byte-identical to this PR:

$ git diff --stat fix/compiled-arcsin-branch ci-check/arcsin-branch
 .github/workflows/CSharpTest.yml | 1 +
 1 file changed, 1 insertion(+)

@Happypig375
Happypig375 merged commit 6e6c969 into ASC-Community:master Aug 4, 2026
24 checks passed
@Rafael-SOWNet
Rafael-SOWNet deleted the fix/compiled-arcsin-branch branch August 4, 2026 20:49
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.

3 participants