Skip to content

fix: avoid slow simplification during multivector display - #598

Merged
utensil merged 4 commits into
pygae:masterfrom
utiberious:perf/display-simp-compat-061
Jul 30, 2026
Merged

fix: avoid slow simplification during multivector display#598
utensil merged 4 commits into
pygae:masterfrom
utiberious:perf/display-simp-compat-061

Conversation

@utiberious

@utiberious utiberious commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Why a replacement PR

#590 fixed the curvilinear-coordinate notebook by selecting trigsimp(method='old') through Simp.profile. That workaround still works and remains unchanged here, but it only protects that example.

#596 tried to make the workaround available throughout galgebra. Its simplification route was too broad: it could affect algebraic operations and change familiar canonical output where there was no performance problem.

The missing distinction was between calculation and presentation. The costly terminal simplify call only needs to be avoided while rendering the specific expression shape known to enter SymPy 1.13's expensive FU traversal.

What this changes

  • Keep ordinary algebraic Simp.apply behavior unchanged.
  • Add a display-only simplification route for multivector string and LaTeX output.
  • Match only a square root or reciprocal square root whose two-term base contains one squared trigonometric function and one squared hyperbolic function.
  • Use trigsimp(method='old') only for that shape on SymPy 1.13 or newer.
  • Respect explicit and in-place Simp.profile changes.

The earlier numerical cost heuristic was removed after benign expressions repeatedly reached the same score through unrelated terms. The existing notebook profile remains; its saved mode is restored by identity rather than through a mutable copy. The notebook note and stored equations are unchanged, preserving the established fraction, termwise-sum, and blade-coefficient forms.

Performance

On SymPy 1.13.3 and 1.14, the representative prolate-spheroidal divergence selects the fallback once and renders the same 434-character result in about 2.5 seconds. The triggering coefficient did not complete within 15 seconds through plain simplify in the same environment.

The latest reviewed false positive now stays on ordinary simplification, completes in about 0.75 seconds on both versions, reduces its rational term, and remains symbolically equivalent. The two earlier false positives remain covered as negative regressions.

Verification

  • focused tests on SymPy 1.12: 14 passed, 1 targeted skip
  • focused tests on SymPy 1.13.3 and 1.14: 15 passed on each
  • full test suite on SymPy 1.13.3 and 1.14: 143 passed, 3 skipped on each
  • LaTeX.ipynb nbval: 22 passed
  • flake8: passed
  • git diff --check: passed
  • notebook stored outputs versus master: no differences

The changelog distinguishes the original example workaround from this display-scoped fallback and corrects the old claim that the slowdown occurred during Ga.build.

Supersedes #596.

@utensil

utensil commented Jul 30, 2026

Copy link
Copy Markdown
Member

Thanks, this is much cleaner than #596. The display-only boundary holds up, and the full suite, SymPy 1.13/1.14 checks, nbval, and unchanged notebooks all pass.

However, the threshold catches an ordinary trig sum where fallback rendering takes 4.32s versus 0.53s with simplify. Please narrow the predicate and add a real, unmocked negative regression.

Also make the boundary test version-aware for SymPy 1.12, avoid mutable default-mode aliasing so explicit profiles remain authoritative.

Changelog is also stale now, it still says it is about Ga.build work.

@utiberious

Copy link
Copy Markdown
Contributor Author

Addressed in b615154.

Full suites pass on SymPy 1.13.3 and 1.14: 141 passed, 3 skipped on each. LaTeX nbval remains 22/22, and notebook stored outputs are unchanged.

@utensil

utensil commented Jul 30, 2026

Copy link
Copy Markdown
Member

Thanks, the SymPy version handling, profile authority, and changelog fixes look good.

There is still something wrong in the predicate. It uses the whole expression’s cost but requires only one small qualifying Pow. With 16 ordinary trig pairs plus sqrt(sin(u) + sinh(v)), it routes to the fallback: 7.67s versus 0.59s for simplify on SymPy 1.14.

Please calculate the cost within each candidate power/base and add this case as a real negative regression.

Everything else looks promising.

@utiberious

utiberious commented Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

Addressed in 41e98be.

The predicate now computes traversal cost inside each qualifying power base. The 16-pair counterexample has a local score of 10, so unrelated terms can no longer push it over the boundary; the prolate sin² + sinh² base scores 18 and still selects the fallback.

The exact counterexample is now a real regression test. On SymPy 1.14 it does not route, completes through the helper in 0.72 s, reduces the rational term, and is symbolically equivalent to the input. The representative prolate display still routes once and renders 434 characters in 2.64 s.

Focused tests pass on SymPy 1.12, 1.13.3, and 1.14. Full suites pass on 1.13.3 and 1.14 with 142 passed and 3 skipped on each; LaTeX nbval remains 22/22. Fresh CI is running on the revised head.

@utiberious
utiberious force-pushed the perf/display-simp-compat-061 branch from 028c051 to 41e98be Compare July 30, 2026 06:08
@utensil

utensil commented Jul 30, 2026

Copy link
Copy Markdown
Member

Thanks, the previous counterexample is fixed and the test, CI, notebook, profile, changelog, and identity checks all hold.

I found another counterexample: sqrt(sin(u) + sinh(v) + z0 + z1 + z2 + z3) also scores 18. With the 16 trig pairs and rational term, the helper takes 5.35s versus 0.72s for simplify and retains the unreduced rational. SymPy 1.13.3 behaves similarly.

Please narrow the signal to the actual squared trig/hyperbolic shape, or scope the fallback to the candidate subexpression, and add this case as a real negative regression. Try your best to make it bounded safely.

@utiberious

Copy link
Copy Markdown
Contributor Author

Addressed in 449c4d2.

The repeated false positives show that a numerical tree score is not a safe signal here, so I removed it rather than tuning another threshold. The fallback now matches only the observed shape: a square root or reciprocal square root whose two-term base is one squared trigonometric function plus one squared hyperbolic function.

The exact new counterexample is a real negative regression. On SymPy 1.13.3 and 1.14 it does not route, completes through ordinary simplification in about 0.75 s, reduces the rational term, and remains symbolically equivalent. The representative prolate expression still routes once and renders the unchanged 434-character result in about 2.5 s.

Focused tests pass on SymPy 1.12, 1.13.3, and 1.14. Full suites pass on 1.13.3 and 1.14 with 143 passed and 3 skipped on each; LaTeX nbval remains 22/22. No notebook file changed. Fresh CI is running on the revised head.

@utensil

utensil commented Jul 30, 2026

Copy link
Copy Markdown
Member

That is possibly the best we can do without going back to use explicit example profile.

Removing the numerical heuristic and matching only the observed squared trig/hyperbolic radical is the right boundary.

It looks that the prior counterexamples stay on ordinary simplify, the prolate case remains bounded with the unchanged rendering, among other things.

@utiberious

Copy link
Copy Markdown
Contributor Author

Final performance summary for 449c4d2, measured with Python 3.12.13 and SymPy 1.14 on an Apple M4 Pro VM. Each non-timeout figure is the median of five isolated processes; “ordinary” uses an explicit [simplify] profile to bypass the fallback while retaining the same display path.

Representative workload PR/default display Ordinary simplify Result
Cartesian derivatives, 7 objects / 282 chars 0.070 s 0.073 s no material change
Spherical derivatives, 8 objects / 764 chars 0.472 s 0.461 s +11 ms
Paraboloidal derivatives, 7 objects / 906 chars 0.202 s 0.196 s +6 ms
Trig sum + rational 0.513 s 0.510 s +4 ms; rational reduced
Trig sum + small mixed radical 0.622 s 0.619 s +3 ms; rational reduced
Trig sum + larger benign radical 0.765 s 0.758 s +7 ms; rational reduced
Prolate divergence, 434 chars 2.468 s timed out after 15.148 s more than 6.1× faster

The five-run PR ranges were 0.069–0.072 s Cartesian, 0.451–0.479 s spherical, 0.197–0.213 s paraboloidal, and 2.448–2.492 s prolate. Across the ordinary cases, median differences were −4.1% to +3.0%, at most 11 ms; five of six timing ranges overlapped.

A route inventory covered 46 coefficient simplifications across the Cartesian, spherical, and paraboloidal samples: none matched. The prolate divergence contributed one coefficient and it matched. All three review-derived false-positive families remained on ordinary simplify.

The complete curvi_linear_latex.py script still uses #590’s explicit old-trigsimp profile and completed in a 4.783 s median over three isolated runs (4.746–4.876 s). This PR does not intercept that profile, and its notebook source and stored rendering remain unchanged.

SymPy 1.13.3 spot checks showed the same behavior: 0.765 s for the latest benign case and 2.546 s for prolate. These figures establish bounded behavior for this representative sample, not every possible symbolic composition. CI and documentation checks are green on the exact head.

@utensil

utensil commented Jul 30, 2026

Copy link
Copy Markdown
Member

The complete curvi_linear_latex.py script still uses https://github.com/pygae/galgebra/pull/590’s explicit old-trigsimp profile and completed in a 4.783 s median over three isolated runs (4.746–4.876 s). This PR does not intercept that profile, and its notebook source and stored rendering remain unchanged.

I don't like that we have to keep this old-trigsimp profile but it seems to add ~10% time if we remove it, and what's worse, the simplified results become less desirable.

I hope we can have one last round to try structural pattern matching on bottlenecks, and develop broader structural matching only alongside #597’s visual gallery and regression audit, where every new family gets explicit mathematical and visual acceptance criteria.

@utiberious

Copy link
Copy Markdown
Contributor Author

I tried one final targeted structural-profile spike. It preserved the complete stored TeX output exactly, but it did not improve on the existing example profile.

The prototype routed 67 of 208 coefficient simplifications: 17/53 spherical, 2/78 paraboloidal, and 48/77 prolate. Fresh-process comparisons found it 3.0% slower than the current profile on SymPy 1.14 and 5.0% slower on 1.13.3. The paraboloidal section was 89% and 106% slower respectively, while prolate was effectively unchanged.

The paraboloidal repeated-radical denominator does have a sound narrow structural match. Reproducing all established prolate forms is the problem: it requires routing every trig/hyperbolic coefficient, including small expressions such as sin(x)**2, rather than matching one pathological shape.

So I am leaving this PR unchanged. The explicit example profile remains the faster presentation policy, while this PR keeps the exact display-only fallback for the observed SymPy slowdown. Broader canonical-form matching, including the paraboloidal family, should proceed with #597's mathematical and visual regression gates.

@utensil

utensil commented Jul 30, 2026

Copy link
Copy Markdown
Member

Thanks, I concur. We defer to #597 to solve this with better signals from the visual regression gate.

@utensil
utensil merged commit a337316 into pygae:master Jul 30, 2026
5 checks passed
@utensil utensil added this to the 0.6.1 milestone Jul 30, 2026
This was referenced Jul 30, 2026
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.

2 participants