An inversion with no written preimage returns null, not an exception - #1510
Merged
Merged
Conversation
Invert and InvertNode return null where the preimage has no written form, and a node that inverts a child passes the child's null on. Where several inversions must all be written, the first null ends it and the rest are never computed, as the exception abandoned them: computing them anyway made SolveHard 3% slower. The analytical solvers answer the set-builder on null, at the level it was met, as they did on the exception, and the substitutions of the exponential and trigonometric solvers go through InvertEach, which declines on null. Review of #1504. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012sonx8iAspMiwRwokT1Ura
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
From the review of #1504 (comment). An inversion that couldn't write its preimage threw
CannotInvertException, and the analytical solvers caught it. It now returnsnull.InvertandInvertNodereturnIEnumerable<Entity>?.nullmeans the preimage has no written form. An empty sequence still means there are provably no roots.nullon. Where several inversions must all be written, such as the two branches ofsinandcos, the roots of 1 for a whole power, or the cases of a boolean node, the firstnullends it, and the parts after it are never computed.AnalyticalEquationSolver.SolveandAnalyticalSetSolver.Solveanswer the set-builder onnull, at the level where it was met, as they did on the exception. So(x - 1) x! = 0is still{ 1 }together with thexfor whichx! = 0.InvertEach, which declines onnull. Their substitutions are powers of the variable, and those always have a written preimage, so this changes no answer.Nullable annotations are on and warnings are errors, so the compiler rejects any caller that doesn't handle
null.Measured
SolveHardallocates 0.7% less: it meets inversions with no written preimage, and those allocated an exception each.SolveHard. It computed every part of a composite inversion before looking for anull, where the exception had abandoned the rest at once. Stopping at the firstnullputs it back: 66.0 ms against 66.9 ms on master, over six alternating runs of each. Solves that meet such an inversion directly, such asx mod 3 = 1,gcd(x, 4) = 2anderf(x) = 1/2, take 2 to 10% less time.🤖 Generated with Claude Code
https://claude.ai/code/session_012sonx8iAspMiwRwokT1Ura