Commit 4103148
authored
fix: use trigsimp in prolate spheroidal example to avoid SymPy 1.13 slowdown (#576)
* fix: use trigsimp in prolate spheroidal example to avoid SymPy 1.13 slowdown (#576)
SymPy 1.13 (PR 26390) added a .replace() traversal in TR3/futrig that scales
badly for mixed trig+hyperbolic expressions. normalize_metric calls Simp.apply
~70 times during Ga.build with norm=True, hitting that path for every
coefficient. The actual metric reduction (sinh²+cosh²→1 etc.) is handled
separately by square_root_of_expr via its own trigsimp call and is unaffected
by the Simp.modes setting.
Scope the fix to this one call site rather than changing the global default:
use Simp.set([trigsimp]) before Ga.build and restore [simplify] after.
This brings the prolate spheroidal build from ~1800s down to ~2s.
* fix: use Simp.profile not Simp.set (correct API)
Simp.profile is the actual method name for overriding simplification
modes; Simp.set does not exist and caused AttributeError in CI.
* fix: use cancel instead of trigsimp to avoid SymPy 1.13 TR3 slowdown
trigsimp also calls futrig/TR3 which has the same .replace() traversal
regression introduced in sympy/sympy PR 26390 (SymPy >= 1.13). Switch
to cancel (polynomial GCD cancellation) which avoids trig traversal
entirely and reduces Ga.build time from ~1800s to ~0.33s with SymPy 1.13.3.
The metric simplification (sinh^2+cosh^2=1 etc.) is handled by
square_root_of_expr's own trigsimp call, which is unaffected.
Fixes #576
* fix(notebooks): update stored output for cancel simplifier in prolate spheroidal example
cancel does not apply double-angle identities (sin(eta)cos(eta) stays
as-is rather than becoming sin(2*eta)/2), so update the stored cell
output to match what cancel actually produces. The previous stored
output was generated with simplify which does apply trig identities.
The two forms are mathematically equivalent; the nbval sanitize config
already handles the left-paren spacing difference between SymPy versions.
* fix: use trigsimp(method='old') to avoid SymPy 1.13 TR3 slowdown (#576)
Switch from cancel to trigsimp(method='old') for the Ga.build call in
derivatives_in_prolate_spheroidal_coordinates. The default simplify and
default trigsimp both route through futrig/TR3 in fu.py, which gained an
expensive .replace() traversal in SymPy 1.13 (PR 26390) causing the
notebook cell to time out after 600 s.
trigsimp(method='old') uses a different code path that avoids that
traversal while still applying double-angle identities
(sin*cos -> sin(2x)/2, sinh*cosh -> sinh(2x)/2), so the canonical
output form is preserved and the stored notebook output does not need
to change.
Also restores the notebook output to its original simplify-based form
and removes the unused cancel import.
* fix: extend fast-simp scope to cover paraboloidal and all Mv printing
The previous fix applied trigsimp(method='old') only around Ga.build for
the prolate spheroidal case, but Mv._sympystr also calls Simp.apply when
printing every multivector. With the profile restored to simplify before
the print statements, all grad*f / grad|A / grad^B calls triggered the
SymPy 1.13 TR3 traversal slowdown. In addition, derivatives_in_
paraboloidal_coordinates used the default simplify throughout.
Fix: move the Simp.profile call to main() so it covers all three
coordinate-system functions AND all subsequent print/Fmt calls.
Use trigsimp(cancel(e), method='old') -- cancel() pre-reduces rational
factors quickly, then trigsimp(method='old') applies double-angle
identities (sin*cos->sin(2x)/2, sinh*cosh->sinh(2x)/2) without the
expensive TR3 traversal. The canonical output form is preserved.
* fix: remove cancel() from fast-simp profile to preserve canonical output form
cancel() applies polynomial GCD reduction that changes the canonical form
even when nothing cancels (e.g. A^theta/tan(theta) becomes a large fraction).
This caused output mismatches in spherical and paraboloidal coordinate outputs
vs. the stored notebook reference.
trigsimp(method='old') alone avoids the SymPy 1.13 TR3/fu.py traversal
slowdown while producing output consistent with the reference notebook.
* fix: patch TR3 in fu.py to avoid SymPy 1.13+ O(N*M) traversal overhead
SymPy PR #26390 added a .replace() traversal inside TR3 (sympy/simplify/fu.py)
to normalize numeric Mul expressions inside trig arguments. For galgebra
curvilinear coordinate expressions the trig arguments are pure symbols, so
the traversal is always a no-op but still imposes O(N*M) overhead on large
expression trees, causing multi-minute slowdowns.
Temporarily monkey-patching TR3 with a version that skips the .replace()
restores pre-1.13 performance while producing identical canonical output for
symbolic arguments. The patch is applied only during the three coordinate
derivative functions and restored unconditionally via try/finally.
* fix: use Simp.profile instead of TR3 monkey-patch to fix SymPy 1.13 slowdown (#576)
Replace the sympy.simplify.fu.TR3 monkey-patch with galgebra's own
Simp.profile API, using trigsimp(method='old') to bypass the slow
fu.py code path introduced in SymPy 1.13 (PR #26390).
Clear stored notebook output for curvi_linear_latex since trigsimp
with method='old' produces equivalent but superficially different
trig forms compared to simplify.
* fix: add normalizers to validate_nb_refresh.py for trigsimp algebraic differences
Add five new LaTeX normalizers to handle the output differences introduced
when using trigsimp(method='old') instead of simplify for the curvilinear
coordinate example:
- _norm_sin2_sinh2_identity: sin²(η)+sinh²(ξ) ↔ -cos²(η)+cosh²(ξ)
- _norm_sum_sqrt_to_power32: (X²+Y²)^{3/2} ↔ X²√(…)+Y²√(…)
- _norm_distribute_r2_denominator: \frac{r²A+rB+C}{r²} ↔ A+B/r+C/r²
- _norm_strip_outer_parens_before_basis: \left(X\right) basis ↔ X basis
- _norm_collapse_spaces: trailing spaces in \frac args
Also re-execute examples/ipython/LaTeX.ipynb with the updated simplifier
so stored outputs match fresh nbval execution.
Document two known remaining algebraically-equivalent differences that
require symbolic algebra (SymPy) to verify: the spherical curl e_r
component and the prolate-spheroidal divergence.
Closes #576
* docs(validate_nb_refresh): document assumption in _norm_strip_outer_parens_before_basis
Adds an explicit Assumption note explaining that the non-greedy .*?
regex relies on galgebra never emitting a bare nested \left(...\right)
group directly before a basis blade — per review feedback on PR #590.
* docs(nb): add SymPy 1.13 workaround note after curvi_linear_latex outputs
Inserts a markdown cell immediately after the check('curvi_linear_latex')
cell documenting the two cosmetic differences from the pre-1.13 canonical
form:
- spherical curl e_r: less-factored fraction vs factored (2A^φ/tan + …)|sin|
- prolate-spheroidal ∇·A: -cos²(η)+cosh²(ξ) vs sin²(η)+sinh²(ξ)
Both are algebraically equivalent. Points readers to issue #576 for
the proper upstream fix tracking.1 parent 336ba2b commit 4103148
3 files changed
Lines changed: 367 additions & 105 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
181 | 181 | | |
182 | 182 | | |
183 | 183 | | |
184 | | - | |
185 | | - | |
186 | | - | |
187 | | - | |
188 | | - | |
189 | | - | |
190 | | - | |
191 | | - | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
192 | 205 | | |
193 | 206 | | |
194 | 207 | | |
| |||
0 commit comments