perf(metric): pre-apply cancel() before Simp.apply to fix SymPy 1.13 slowdown (#576)#591
Open
utiberious wants to merge 2 commits intopygae:masterfrom
Open
perf(metric): pre-apply cancel() before Simp.apply to fix SymPy 1.13 slowdown (#576)#591utiberious wants to merge 2 commits intopygae:masterfrom
utiberious wants to merge 2 commits intopygae:masterfrom
Conversation
…c and Christoffel_symbols SymPy 1.13 introduced a slow .replace() traversal in TR3/fu.py that is triggered by both simplify() and trigsimp() (default method). Simp.apply is called ~72 times during Ga.build(..., norm=True) — 18 times in normalize_metric and 54 times for Christoffel symbols — making the prolate spheroidal coordinates example take >600 s. Wrapping each expression with cancel() before passing it to Simp.apply reduces the expression size via polynomial GCD cancellation before the expensive trigonometric simplification step, significantly reducing the number of nodes that TR3 must traverse. This is the library-level fix for issue pygae#576 (option 2), complementing the minimal example-file fix in PR pygae#590.
… change cancel() on purely polynomial Christoffel symbol expressions can change their representation (e.g. expanded 4G^2M^2 - 4GMc^2r + c^4r^2 becomes 4G^2M^2 + c^2r(-4GM + c^2r)) because cancel() uses Poly arithmetic internally, which may normalize polynomials differently. When simplify() then runs on the altered form it produces a different output, breaking the stored gr_metrics notebook output. The cancel() pre-pass is retained only in normalize_metric, where the inputs are genuine fractions (divided by e_norm) rather than polynomial sums, so cancel() there reduces actual rational factors without changing the polynomial normal form.
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
1 participant
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.
Summary
Fixes #576 at the library level by pre-applying
cancel()before eachSimp.apply()call innormalize_metricandChristoffel_symbols.cancel()performs polynomial GCD cancellation to reduce expression sizesimplify()(or any user-configured mode) traverses it.replace()traversal inTR3/fu.py, triggered by bothsimplify()andtrigsimp()-- reducing node count before that traversal cuts runtime dramaticallyAffected call sites (~72 total during
Ga.build(..., norm=True)):normalize_metric: 18 calls (9deentries + 9gentries)Christoffel_symbolsmode 1: 27 callsChristoffel_symbolsmode 2: 27 callsRelation to PR #590
PR #590 is a minimal fix scoped to the example file (
curvi_linear_latex.py) that swapssimplify->trigsimp(method='old')around the slowGa.buildcall. This PR is the library-level complement ("option 2") that benefits all coordinate systems and user code, not just that one example.Test plan
derivatives_in_prolate_spheroidal_coordinates()completes well within the 600 s notebook timeoutsin(2η)/2preserved --canceldoes not apply trig identities)