Skip to content

Speed up the eigensolver and the mean curvature flow - #128

Open
m-reuter wants to merge 3 commits into
Deep-MI:mainfrom
m-reuter:speed
Open

m-reuter wants to merge 3 commits into
Deep-MI:mainfrom
m-reuter:speed

Conversation

@m-reuter

@m-reuter m-reuter commented Oct 19, 2026 •

Copy link
Copy Markdown
Member

Speeds up two computations behind spectral sphere maps: the eigensolver and the mean curvature flow. tria_spherical_project gains from the faster flow right away. It keeps the strict eigensolver tolerance for now: a loose tolerance needs a few extra eigenpairs to avoid missing a repeated eigenvalue, so it goes into the follow-up PR that reworks the sphere map, together with tests.

Changes

  • Solver.eigs has a new tol argument, passed to ARPACK. The default 0 keeps machine precision. Eigenfunctions used as coordinates need far less: with tol=1e-6, the eigensolve on a cortical surface (143k vertices) takes about half the time, and the first eigenvectors span the same space to 1e-8.
  • tria_mean_curvature_flow factorizes only the system matrix of the first step. The matrix changes little between steps, so later steps are solved with conjugate gradients preconditioned by that factorization; a step that does not converge falls back to a direct solve. The result matches solving every step directly to about 1e-11, and the flow is 2 to 3 times faster on a cortical surface, for 3 as well as 30 steps. This works with both backends (use_cholmod).
  • The SciPy minimum rises to 1.12, the first version where cg takes rtol.
  • Code style CI: bibclean 1.0 replaced the bibclean-check command with bibclean check and added rules for page ranges and month names. The workflow uses the new command, the style extra requires bibclean>=1.0, and doc/references.bib is reformatted with bibclean fix.

Tests

A new test flows a cube surface for 8 steps and compares it with solving every step directly. It fails (difference 4e-4) when later steps reuse the first factorization without the conjugate gradient solve. The full suite passes with the current dependencies and with the minimum versions (numpy 1.23.2, SciPy 1.12.0).

@codecov

codecov Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 78.26087% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 67.10%. Comparing base (54dca7e) to head (286226b).

Files with missing lines Patch % Lines
lapy/diffgeo.py 78.26% 4 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #128      +/-   ##
==========================================
+ Coverage   67.05%   67.10%   +0.04%     
==========================================
  Files          19       19              
  Lines        3573     3590      +17     
  Branches      460      464       +4     
==========================================
+ Hits         2396     2409      +13     
- Misses        994      998       +4     
  Partials      183      183              
Flag Coverage Δ
unittests 67.10% <78.26%> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ 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.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Address the CG fallback reuse, thread eigensolver tolerance into the spectral sphere path, and tighten the regression assertion.

2 open issues
What changed in this PR

This pull request improves eigensolver and mean-curvature-flow performance while updating dependency and formatting tooling.

Changes:

  • Adds configurable ARPACK tolerance support.
  • Uses preconditioned CG and factorization reuse for curvature flow.
  • Updates SciPy, bibclean, CI, bibliography, and regression tests.
File Description
pyproject.toml Raises SciPy and bibclean minimum versions.
lapy/​utils/​tests/​test_diffgeo.py Adds curvature-flow accuracy coverage.
lapy/​solver.py Adds the eigensolver tolerance parameter.
lapy/​diffgeo.py Optimizes curvature-flow linear solves.
doc/​references.bib Applies bibclean formatting.
.github/​workflows/​code-style.yml Uses the new bibclean command.

Review effort: Lite

Comment thread lapy/diffgeo.py Outdated
Comment thread lapy/solver.py

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Add regression coverage confirming Solver.eigs forwards tol and preserves the default.

No open issues 🎉

2 resolved since last review

Review effort: Lite

This branch has not been deployed

No deployments
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