Skip to content

perf: compute the COFE solver's up/down pair by a single recursion#499

Merged
Kaptch merged 4 commits into
leanprover-community:masterfrom
sgraf812:cofesolver-updown
Jul 7, 2026
Merged

perf: compute the COFE solver's up/down pair by a single recursion#499
Kaptch merged 4 commits into
leanprover-community:masterfrom
sgraf812:cofesolver-updown

Conversation

@sgraf812

@sgraf812 sgraf812 commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Description

perf: compute the up/down pair by a single recursion. Fix.Impl.up and Fix.Impl.down were mutually recursive, so the compiled code for either hom at level k recomputes both branches at every level, which is 2^k recursive calls. updown computes the section/retraction pair at every level by a single recursion and up/down are its projections, making that O(k). All solver proofs survive unchanged up to structure eta. Measured on a round-trip of a value through level k of a tower over ProdOF (HomOF (LaterOF IdOF) (LaterOF IdOF)) (constOF (LeibnizO Nat)) in the interpreter: the mutual recursion costs 84ms at k = 14 and quadruples every two levels (6.4s at k = 20); the paired recursion stays around a millisecond even at k = 2000. Instrumenting the functor's map counts 2k(k+1) hom constructions with the pair against roughly 4 * 2^k without.

The regression test Iris.Tests.COFESolverEval guards exactly this cost: it round-trips a value through level 21 of that tower at runtime (upN then downN) and reads back a Nat threaded through the constOF component, so the homs must actually be constructed by the compiled code. Compiled evaluation is not subject to maxHeartbeats, so the check is a wall-clock budget calibrated against a pure baseline fold timed in the same interpreter, which makes machine speed cancel out: the paired recursion costs 1 baseline units, the mutual recursion 300 or more, and the limit is 20. A regression therefore fails deterministically in well under a minute instead of hanging the build. The file is not a module and is built by the IrisTest glob rather than listed in Iris.Tests.

Checklist

  • My code follows the mathlib naming and code style conventions
  • I have added my name to the authors section of any appropriate files

@Kaptch

Kaptch commented Jul 5, 2026

Copy link
Copy Markdown
Collaborator

Thanks! I'll take a look at all three ~early next week :)

@Kaptch
Kaptch self-requested a review July 5, 2026 11:31
@sgraf812

sgraf812 commented Jul 5, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for giving it a look, but no rush! This was all produced by Claude and I need to review it myself. #497 and #498 are ready for review, but no need for you to read this one unless I have removed draft status.

sgraf812 added 2 commits July 5, 2026 14:50
Fix.Impl.up and Fix.Impl.down were mutually recursive, so the compiled
code for either hom at level k+1 recomputes both branches at every
level, which is 2^k recursive calls. Computing the pair at every level
by a single recursion makes that O(k). All solver proofs are unchanged
up to structure eta.
Iris.Tests.COFESolverEval round-trips a value through tower level 21 at
runtime and reads back a Nat payload, so the up/down homs must actually
be constructed by the compiled code. The cost is checked against a pure
baseline computation timed in the same interpreter, which makes the
budget machine-independent: the paired recursion costs a few baseline
units, while recomputing the two branches at every level costs several
hundred and fails the budget deterministically instead of hanging the
build. The depth and baseline size can be overridden via environment
variables for experiments.
@sgraf812
sgraf812 force-pushed the cofesolver-updown branch from e2a1f7d to d319365 Compare July 5, 2026 12:53
@sgraf812

sgraf812 commented Jul 5, 2026

Copy link
Copy Markdown
Contributor Author

Not very convinced about the "perf relative to baseline" approach due to flakiness in CI, but I suppose we can always deactivate the test when it becomes a problem.

@sgraf812
sgraf812 marked this pull request as ready for review July 5, 2026 20:43
@Kaptch

Kaptch commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator

All three look good (this one is especially appreciated)! May I ask you only to remove the tests in all three? I think the clarifying comments in the code are enough to understand the rationale. After that, I can merge them asap.

@sgraf812

sgraf812 commented Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

Great! Removed the tests.

@Kaptch
Kaptch merged commit 5558f9f into leanprover-community:master Jul 7, 2026
5 checks passed
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