Skip to content

[Merged by Bors] - feat: support products and disjoint unions in the differential geometry elaborators#30463

Closed
grunweg wants to merge 40 commits intoleanprover-community:masterfrom
grunweg:diffgeo-custom-elaborators-products-binary
Closed

[Merged by Bors] - feat: support products and disjoint unions in the differential geometry elaborators#30463
grunweg wants to merge 40 commits intoleanprover-community:masterfrom
grunweg:diffgeo-custom-elaborators-products-binary

Conversation

@grunweg
Copy link
Contributor

@grunweg grunweg commented Oct 12, 2025

Add support for inferring a model with corners on

  • products of manifolds
  • disjoint unions of manifolds
  • an open subset of a manifold

Note that these can be nested (e.g., a product with multiple factors, a disjoint union of four manifolds or combinations of these).

For binary products of normed spaces, we intentionally refuse to infer a model with corners, as there are two different possible options which are propositionally, but not definitionally equal. For now, we emit a warning to this effect instead. A future PR may remove it (and infer one of these variants): see zulip discussion.

This PR completes an important milestone: models with corners are now inferred for all instances mathlib knows about! Update the module doc-string accordingly.
A future PR will golf mathlib using these elaborators.


Open in Gitpod

@grunweg
Copy link
Contributor Author

grunweg commented Oct 12, 2025

Next step: update the golfing PR to also incorporate this one, try golfing the heck out of mathlib and re-benchmark.

@github-actions
Copy link

github-actions bot commented Oct 12, 2025

PR summary 2d973f3355

Import changes for modified files

No significant changes to the import graph

Import changes for all files
Files Import difference

Declarations diff

+ FindModelResult
+ NormedSpaceInfo
+ findModelInner
+ instance : Coe Expr FindModelResult
- findModel

You can run this locally as follows
## summary with just the declaration names:
./scripts/declarations_diff.sh <optional_commit>

## more verbose report:
./scripts/declarations_diff.sh long <optional_commit>

The doc-module for scripts/declarations_diff.sh contains some details about this script.


No changes to technical debt.

You can run this locally as

./scripts/technical-debt-metrics.sh pr_summary
  • The relative value is the weighted sum of the differences with weight given by the inverse of the current value of the statistic.
  • The absolute value is the relative value divided by the total sum of the inverses of the current values (i.e. the weighted average of the differences).

@grunweg grunweg force-pushed the diffgeo-custom-elaborators-products-binary branch from 42838e8 to 3b4d533 Compare October 12, 2025 13:08
@mathlib4-dependent-issues-bot mathlib4-dependent-issues-bot added the blocked-by-other-PR This PR depends on another PR (this label is automatically managed by a bot) label Oct 12, 2025
@mathlib4-merge-conflict-bot mathlib4-merge-conflict-bot added the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Oct 14, 2025
@mathlib4-merge-conflict-bot
Copy link
Collaborator

This pull request has conflicts, please merge master and resolve them.

@grunweg grunweg force-pushed the diffgeo-custom-elaborators-products-binary branch from 3b4d533 to 5d3c65b Compare October 25, 2025 10:01
@github-actions github-actions bot removed the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Oct 25, 2025
@grunweg grunweg force-pushed the diffgeo-custom-elaborators-products-binary branch 2 times, most recently from 27a2e14 to b532d0b Compare October 25, 2025 10:39
@grunweg grunweg changed the title feat: support binary products in the differential geometry elaborators feat: support binary products and sums in the differential geometry elaborators Oct 25, 2025
@thorimur thorimur self-assigned this Oct 27, 2025
@grunweg grunweg changed the title feat: support binary products and sums in the differential geometry elaborators feat: support products and disjoint unions in the differential geometry elaborators Oct 27, 2025
@grunweg grunweg added t-meta Tactics, attributes or user commands t-differential-geometry Manifolds etc labels Oct 27, 2025
@mathlib4-merge-conflict-bot mathlib4-merge-conflict-bot added the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Oct 28, 2025
@mathlib4-merge-conflict-bot
Copy link
Collaborator

This pull request has conflicts, please merge master and resolve them.

@grunweg grunweg force-pushed the diffgeo-custom-elaborators-products-binary branch from 8cce5e9 to 02f877b Compare November 12, 2025 22:36
@github-actions github-actions bot removed the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Nov 12, 2025
@mathlib4-merge-conflict-bot mathlib4-merge-conflict-bot added the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Nov 19, 2025
@mathlib4-merge-conflict-bot
Copy link
Collaborator

This pull request has conflicts, please merge master and resolve them.

@grunweg
Copy link
Contributor Author

grunweg commented Jan 24, 2026

I have just updated this work on top of the current state in #30744 (which is now maintainer merged, hence will hopefully land soon). #30421 contains the current code; I will overwrite this branch accordingly once this PR is unblocked.

@mathlib4-dependent-issues-bot mathlib4-dependent-issues-bot removed the blocked-by-other-PR This PR depends on another PR (this label is automatically managed by a bot) label Jan 28, 2026
These are not exhaustive, but probably a good start.
…ct space,

return that space and its base field

A future commit will use this to warn on the product of two models with corners
on normed spaces (as there are two potential models on it, which are not
definitionally equal).
The inner loop handles finding a model on a non-product, and the
outer loop will just be responsible for products, disjoint unions
and open subsets of a manifold.
@thorimur
Copy link
Collaborator

thorimur commented Feb 18, 2026

Let me add one bundle of review suggestions + questions for now, all centered on isCLMReducibleDefeqCoefficients:

  • isCLMReducibleDefeqCoefficients, returning three Exprs, is riiiight on the edge of needing a structure return type, I feel! :) I've marked it private for now, seeing as it only has two usages.
  • I think it can return the Exprs instead of wrapping this in an Option, as in both of its uses, we throw approximately the same error (up to the word "either"). Are there cases you anticipate that make the Option useful?
  • I changed indentation, and used pureIsDefEq instead of isDefEq to avoid assigning mvars.
  • I added a check that the map was a RingHom.id, seeing as that was a TODO. Are there cases that make this more complicated? (Coercions or such?) Feel free to revert this change if so (and just leave a quick comment explaining why it's a todo/what the blocking issue is :) )

Please take the commit here as a companion to the comment showing the discussed changes, rather than a final version of the code :)

Copy link
Collaborator

@thorimur thorimur left a comment

Choose a reason for hiding this comment

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

I think this is really close. :) I wanted to give you some more review comments to act on immediately if helpful.

@grunweg
Copy link
Contributor Author

grunweg commented Feb 18, 2026

Let me add one bundle of review suggestions + questions for now, all centered on isCLMReducibleDefeqCoefficients:

* `isCLMReducibleDefeqCoefficients`, returning three `Expr`s, is riiiight on the edge of needing a structure return type, I feel! :) I've marked it `private` for now, seeing as it only has two usages.

* I think it can return the `Expr`s instead of wrapping this in an `Option`, as in both of its uses, we throw approximately the same error (up to the word "either"). Are there cases you anticipate that make the `Option` useful?

* I changed indentation, and used `pureIsDefEq` instead of `isDefEq` to avoid assigning mvars.

* I added a check that the map was a `RingHom.id`, seeing as that was a TODO. Are there cases that make this more complicated? (Coercions or such?) Feel free to revert this change if so (and just leave a quick comment explaining why it's a todo/what the blocking issue is :) )

Please take the commit here as a companion to the comment showing the discussed changes, rather than a final version of the code :)

Thanks a lot for the review (as usual). There is actually #35176, which addresses the exact same TODO :-)
Actually, how about I roll your changes into that PR, you take a quick look if you like them, and then we get that PR in first?

@thorimur
Copy link
Collaborator

Sounds good (re: rolling changes into that PR)! :)

grunweg and others added 4 commits February 18, 2026 22:57
Co-authored-by: Thomas R. Murrills <68410468+thorimur@users.noreply.github.com>
Co-authored-by: Thomas R. Murrills <68410468+thorimur@users.noreply.github.com>
@mathlib-dependent-issues mathlib-dependent-issues bot added blocked-by-other-PR This PR depends on another PR (this label is automatically managed by a bot) and removed blocked-by-other-PR This PR depends on another PR (this label is automatically managed by a bot) labels Feb 18, 2026
Copy link
Collaborator

@thorimur thorimur left a comment

Choose a reason for hiding this comment

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

Looks good to me after these changes! :) (And after the TODO in the last commit!)

grunweg and others added 2 commits February 19, 2026 23:59
Co-authored-by: Thomas R. Murrills <68410468+thorimur@users.noreply.github.com>
@grunweg
Copy link
Contributor Author

grunweg commented Feb 19, 2026

Thanks for your comments again - all addressed.

@thorimur
Copy link
Collaborator

Wonderful! Looking forward to this finally being merged! :)

maintainer merge

@github-actions
Copy link

🚀 Pull request has been placed on the maintainer queue by thorimur.

@mathlib-triage mathlib-triage bot added the maintainer-merge A reviewer has approved the changed; awaiting maintainer approval. label Feb 20, 2026
@ocfnash
Copy link
Contributor

ocfnash commented Feb 23, 2026

Thank you both!

bors merge

@mathlib-triage mathlib-triage bot added ready-to-merge This PR has been sent to bors. and removed maintainer-merge A reviewer has approved the changed; awaiting maintainer approval. labels Feb 23, 2026
mathlib-bors bot pushed a commit that referenced this pull request Feb 23, 2026
…ry elaborators (#30463)

Add support for inferring a model with corners on
- products of manifolds
- disjoint unions of manifolds
- an open subset of a manifold

Note that these can be nested (e.g., a product with multiple factors, a disjoint union of four manifolds or combinations of these).

For binary products of normed spaces, we intentionally refuse to infer a model with corners, as there are two different possible options which are propositionally, but not definitionally equal. For now, we emit a warning to this effect instead. A future PR may remove it (and infer one of these variants): see [zulip discussion](https://leanprover.zulipchat.com/#narrow/channel/287929-mathlib4/topic/Differential.20geometry.20elaborators.20experiment/near/544942546).

This PR completes an important milestone: models with corners are now inferred for *all* instances mathlib knows about! Update the module doc-string accordingly.
A future PR will golf mathlib using these elaborators.

Co-authored-by: thorimur <68410468+thorimur@users.noreply.github.com>
Co-authored-by: thorimur <thomasmurrills@gmail.com>
@mathlib-bors
Copy link
Contributor

mathlib-bors bot commented Feb 23, 2026

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title feat: support products and disjoint unions in the differential geometry elaborators [Merged by Bors] - feat: support products and disjoint unions in the differential geometry elaborators Feb 23, 2026
@mathlib-bors mathlib-bors bot closed this Feb 23, 2026
pfaffelh pushed a commit to pfaffelh/mathlib4 that referenced this pull request Mar 2, 2026
…ry elaborators (leanprover-community#30463)

Add support for inferring a model with corners on
- products of manifolds
- disjoint unions of manifolds
- an open subset of a manifold

Note that these can be nested (e.g., a product with multiple factors, a disjoint union of four manifolds or combinations of these).

For binary products of normed spaces, we intentionally refuse to infer a model with corners, as there are two different possible options which are propositionally, but not definitionally equal. For now, we emit a warning to this effect instead. A future PR may remove it (and infer one of these variants): see [zulip discussion](https://leanprover.zulipchat.com/#narrow/channel/287929-mathlib4/topic/Differential.20geometry.20elaborators.20experiment/near/544942546).

This PR completes an important milestone: models with corners are now inferred for *all* instances mathlib knows about! Update the module doc-string accordingly.
A future PR will golf mathlib using these elaborators.

Co-authored-by: thorimur <68410468+thorimur@users.noreply.github.com>
Co-authored-by: thorimur <thomasmurrills@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-merge This PR has been sent to bors. t-differential-geometry Manifolds etc t-meta Tactics, attributes or user commands

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants