Skip to content

[Merged by Bors] - feat: Define AnalyticManifold as a charted space compatible with analyticGroupoid#10853

Closed
girving wants to merge 13 commits intomasterfrom
GI_manifold
Closed

[Merged by Bors] - feat: Define AnalyticManifold as a charted space compatible with analyticGroupoid#10853
girving wants to merge 13 commits intomasterfrom
GI_manifold

Conversation

@girving
Copy link
Copy Markdown
Collaborator

@girving girving commented Feb 22, 2024

  1. Change the definition of analyticGroupoid to mean analytic everywhere including on the boundary, using the recently added AnalyticWithinAt def.
  2. Define AnalyticManifold as a charted space compatible with analyticGroupoid.

In finite dimensions AnalyticManifold is equivalent to SmoothManifoldWithCorners, so an alternative option would be to leave this out and prove the necessary correspondence via Osgood's theorem. However, we don't have Osgood's theorem yet, and it's nice to have a version that works arbitrarily.


Open in Gitpod

@girving girving added the t-differential-geometry Manifolds etc label Feb 22, 2024
@girving
Copy link
Copy Markdown
Collaborator Author

girving commented Feb 22, 2024

To foreshadow, the next thing I need to define is manifold-analytic maps. Currently I have these defined at https://github.com/girving/ray/blob/be98efd5d2fb6dad741c905262a93bb9aa6eaf49/Ray/AnalyticManifold/AnalyticManifold.lean#L248 only in the boundaryless case, which means analytic in charts.

We’ll need to choose how to extend this to the boundaries. The natural thing following analyticGroupoid is analytic on interior points and smooth at boundary points. If we think that’s the right definition and do the refactor and make a follow up PR, but I’ll wait for discussion first.

There’s some awkwardness in that basic theory if we require only smoothness at boundary points won’t look like normal analytic functions. In particular, an analytic function at a point is locally analytic, but this wouldn’t be true if HolomorphicAt at a boundary point meant smooth there. So we might want to do define HolomorphicAt at boundaries to mean smooth at the boundary and holomorphic at nearby interior points.

I’ll note that the “smooth at boundary” setup makes analyticGroupoid nontrivially more complex, and I’m not sure what it buys us: I haven’t seen that definition before, and currently analyticGroupoid is unused in mathlib.

@sgouezel
Copy link
Copy Markdown
Contributor

sgouezel commented Mar 6, 2024

I have to say that I don't really understand why we use this definition of analytic groupoid instead of the more direct definition "analytic at every point, including the boundary". Do you understand what the more complicated definition buys us?

@girving
Copy link
Copy Markdown
Collaborator Author

girving commented Mar 6, 2024

No, I don't understand what the more complicated definition buys us. I haven't seen it elsewhere in mathematics: it's common to want "analytic inside, continuous at the boundary", but smooth I haven't seen (and I think analytic inside, continuous at the boundary is better named something else).

If we want to do analytic at the boundary we need some additional definitions, though, since we don't have AnalyticAtWithin. Though that is straightforward to write down: it's just "has a power series which matches the function within", or equivalently "is locally the restriction of an analytic function. In either case, the definition would be downstream of AnalyticAt in a separate file.

I'd be happy to make that change if people think it's the right thing.

@girving
Copy link
Copy Markdown
Collaborator Author

girving commented Mar 6, 2024

Here's the PR where it was added: #6386. I don't see discussion there of the smoothness part, but I may have just failed to expand the right comment thread. Cc @michaellee94.

@michaellee94
Copy link
Copy Markdown
Collaborator

michaellee94 commented Mar 6, 2024

Thanks for the ping, @girving. I seem to recall having some difficulty with the pregroupoid definition when I tried to extend analyticity to the boundary (this was only my second PR, so I'm still a mathlib neophyte). If you have a working implementation, I would love to take a look.

Additionally, my intention was to re-implement the existing examples of smooth manifolds (the interval, the sphere) in the analytic setting--I've changed jobs recently and haven't had the time, but I still think it would be valuable.

@girving
Copy link
Copy Markdown
Collaborator Author

girving commented Mar 6, 2024

The sphere is https://github.com/girving/ray/blob/main/Ray/AnalyticManifold/RiemannSphere.lean, and any reasonable subset of an AnalyticManifold should be a submanifold (including the interval, for real analytic).

So from your perspective, if I can make AnalyticGroupoid work with analytic at the boundary, that change is fine?

@michaellee94
Copy link
Copy Markdown
Collaborator

michaellee94 commented Mar 6, 2024

The sphere is https://github.com/girving/ray/blob/main/Ray/AnalyticManifold/RiemannSphere.lean

Very nice!

So from your perspective, if I can make AnalyticGroupoid work with analytic at the boundary, that change is fine?

Certainly, at least for me.

@girving
Copy link
Copy Markdown
Collaborator Author

girving commented Mar 12, 2024

I've updated the definition to require analyticity including on the boundary. There are two main warts:

  1. AnalyticWithinOn is an annoying name, but unfortunately it's not the same as the current AnalyticOn.
  2. It is convenient in many of the proofs about analyticity within to use equivalence to a local extension being analytic. But unfortunately this is only true if the space is complete, so a lot of the lemmas require completeness unnecessarily.

@ghost ghost added the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Mar 20, 2024
@github-actions
Copy link
Copy Markdown

github-actions bot commented Jun 9, 2024

PR summary f2d9f2b4c0

Import changes for modified files

Dependency changes

File Base Count Head Count Change
Mathlib.Geometry.Manifold.AnalyticManifold 1549 1551 +2 (+0.13%)
Import changes for all files
Files Import difference
Mathlib.Geometry.Manifold.AnalyticManifold 2

Declarations diff

+ AnalyticManifold
+ AnalyticManifold.prod
+ AnalyticManifold.self
+ AnalyticManifold.smoothManifoldWithCorners
+ analyticGroupoid_prod
+ analyticPregroupoid
+ mem_analyticGroupoid

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 script/declarations_diff.sh contains some details about this script.

@ghost ghost removed the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Jun 9, 2024
@girving
Copy link
Copy Markdown
Collaborator Author

girving commented Jun 10, 2024

@sgouezel Wow, thank you for resurrecting this! Let me know if I can help, though it seems like it’s passing now.

Copy link
Copy Markdown
Contributor

@grunweg grunweg left a comment

Choose a reason for hiding this comment

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

Thanks for your PR; I'm still excited to see it! I wonder how this could get merged best.

For instance: this PR is quite large; I think splitting into several parts could speed up the review. The zero radius and constFormalMultilinearSeries_zero] look good; if you make a PR for those, I'd approve that quickly. Perhaps, the Analytic/Within file could also be a separate PR.

Edited to add: I realised that simply moving the definition of AnalyticGroupoid could be another quick PR.
Reviewing time is very much super-linear in the PR size, so splitting off easy pieces makes this job easier and usually faster.

girving added a commit that referenced this pull request Jun 12, 2024
This is a few isolated lemmas split out of
#10853.

1. AnalyticAt.comp_of_eq
2. FormalMultilinearSeries.zero_radius
3. constFormalMultilinearSeries_zero
@girving
Copy link
Copy Markdown
Collaborator Author

girving commented Jun 12, 2024

@grunweg: #13784 splits out the isolated lemmas, and #13785 moves analyticGroupoid to the new file.

girving added a commit that referenced this pull request Jun 12, 2024
I'm about to rewrite the definition of `analyticGroupoid` and define
`AnalyticManifold` on top of it, in
#10853. @grunweg
suggested splitting out the move of `analyticGroupoid` into a separate
commit, so that the diff is easier. It used to be in
`SmoothManifoldWithCorners.lean`, but this makes no sense as nothing
downstream of that file uses `analyticGroupoid`.
@girving girving added blocked-by-other-PR This PR depends on another PR (this label is automatically managed by a bot) and removed awaiting-review labels Jun 12, 2024
@ghost ghost 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 Jun 12, 2024
mathlib-bors bot pushed a commit that referenced this pull request Jun 14, 2024
…#13785)

It used to be in `SmoothManifoldWithCorners.lean`, but this makes no sense as nothing downstream of that file uses `analyticGroupoid`. In #10853, I will rewrite the definition of `analyticGroupoid` and define `AnalyticManifold` on top of it.
@ghost ghost added the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Jun 14, 2024
mathlib-bors bot pushed a commit that referenced this pull request Jun 19, 2024
…use (#13784)

This is a few isolated lemmas split out of
#10853.

1. AnalyticAt.comp_of_eq
2. FormalMultilinearSeries.zero_radius
3. constFormalMultilinearSeries_zero
@ghost ghost removed the blocked-by-other-PR This PR depends on another PR (this label is automatically managed by a bot) label Jun 19, 2024
@girving girving removed the awaiting-author A reviewer has asked the author a question or requested changes. label Aug 3, 2024
@girving
Copy link
Copy Markdown
Collaborator Author

girving commented Aug 3, 2024

@grunweg Sorry for the delay merging those; I missed the notification somehow.

@girving
Copy link
Copy Markdown
Collaborator Author

girving commented Aug 20, 2024

@grunweg Are you still up for reviewing this, or we find a different reviewer?

Copy link
Copy Markdown
Contributor

@grunweg grunweg left a comment

Choose a reason for hiding this comment

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

@girving I'm very very sorry for the delay. I took a detailed look now.

Your PR looks very good to me. I have a number of comments, but they are all very minor (mostly, about compressing your code slightly). Mostly, you can simply accept the github suggestions; if you prefer, I can also make these tweaks tomorrow.
I like how the proofs about the analytic groupoid became much simpler - partly because of the simpler definition, but also because you golfed the proofs! I found a few more ways to compress your code; should be really quick. (If you prefer, I can also apply these changes myself tomorrow; let me know!)

Can you update the commit message, please - and make sure to

  • mention the change of definition and some words on why
  • add that you prove the product of analytic manifolds to be analytic? Thanks!

@grunweg grunweg added the awaiting-author A reviewer has asked the author a question or requested changes. label Aug 20, 2024
girving and others added 9 commits August 21, 2024 11:31
Co-authored-by: grunweg <rothgami@math.hu-berlin.de>
Co-authored-by: grunweg <rothgami@math.hu-berlin.de>
Co-authored-by: grunweg <rothgami@math.hu-berlin.de>
Co-authored-by: grunweg <rothgami@math.hu-berlin.de>
Co-authored-by: grunweg <rothgami@math.hu-berlin.de>
Co-authored-by: grunweg <rothgami@math.hu-berlin.de>
Co-authored-by: grunweg <rothgami@math.hu-berlin.de>
@girving
Copy link
Copy Markdown
Collaborator Author

girving commented Aug 21, 2024

Most comments addressed! I'll await replies to the last two.

Copy link
Copy Markdown
Contributor

@grunweg grunweg left a comment

Choose a reason for hiding this comment

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

Thanks for the prompt response!
maintainer merge

@github-actions
Copy link
Copy Markdown

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

@github-actions github-actions bot added the maintainer-merge A reviewer has approved the changed; awaiting maintainer approval. label Aug 21, 2024
@girving girving removed the awaiting-author A reviewer has asked the author a question or requested changes. label Aug 21, 2024
@girving
Copy link
Copy Markdown
Collaborator Author

girving commented Aug 21, 2024

@grunweg I fixed the commit message.

@sgouezel
Copy link
Copy Markdown
Contributor

bors r+
Thanks!

@github-actions github-actions bot added the ready-to-merge This PR has been sent to bors. label Aug 22, 2024
mathlib-bors bot pushed a commit that referenced this pull request Aug 22, 2024
…nalyticGroupoid` (#10853)

1. Change the definition of `analyticGroupoid` to mean analytic everywhere including on the boundary, using the recently added `AnalyticWithinAt` def.
2. Define `AnalyticManifold` as a charted space compatible with `analyticGroupoid`. 

In finite dimensions `AnalyticManifold` is equivalent to `SmoothManifoldWithCorners`, so an alternative option would be to leave this out and prove the necessary correspondence via Osgood's theorem. However, we don't have Osgood's theorem yet, and it's nice to have a version that works arbitrarily.
@mathlib-bors
Copy link
Copy Markdown
Contributor

mathlib-bors bot commented Aug 22, 2024

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title feat: Define AnalyticManifold as a charted space compatible with analyticGroupoid [Merged by Bors] - feat: Define AnalyticManifold as a charted space compatible with analyticGroupoid Aug 22, 2024
@mathlib-bors mathlib-bors bot closed this Aug 22, 2024
@mathlib-bors mathlib-bors bot deleted the GI_manifold branch August 22, 2024 06:45
bjoernkjoshanssen pushed a commit that referenced this pull request Sep 9, 2024
…nalyticGroupoid` (#10853)

1. Change the definition of `analyticGroupoid` to mean analytic everywhere including on the boundary, using the recently added `AnalyticWithinAt` def.
2. Define `AnalyticManifold` as a charted space compatible with `analyticGroupoid`. 

In finite dimensions `AnalyticManifold` is equivalent to `SmoothManifoldWithCorners`, so an alternative option would be to leave this out and prove the necessary correspondence via Osgood's theorem. However, we don't have Osgood's theorem yet, and it's nice to have a version that works arbitrarily.
bjoernkjoshanssen pushed a commit that referenced this pull request Sep 9, 2024
…nalyticGroupoid` (#10853)

1. Change the definition of `analyticGroupoid` to mean analytic everywhere including on the boundary, using the recently added `AnalyticWithinAt` def.
2. Define `AnalyticManifold` as a charted space compatible with `analyticGroupoid`. 

In finite dimensions `AnalyticManifold` is equivalent to `SmoothManifoldWithCorners`, so an alternative option would be to leave this out and prove the necessary correspondence via Osgood's theorem. However, we don't have Osgood's theorem yet, and it's nice to have a version that works arbitrarily.
bjoernkjoshanssen pushed a commit that referenced this pull request Sep 12, 2024
…nalyticGroupoid` (#10853)

1. Change the definition of `analyticGroupoid` to mean analytic everywhere including on the boundary, using the recently added `AnalyticWithinAt` def.
2. Define `AnalyticManifold` as a charted space compatible with `analyticGroupoid`. 

In finite dimensions `AnalyticManifold` is equivalent to `SmoothManifoldWithCorners`, so an alternative option would be to leave this out and prove the necessary correspondence via Osgood's theorem. However, we don't have Osgood's theorem yet, and it's nice to have a version that works arbitrarily.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintainer-merge A reviewer has approved the changed; awaiting maintainer approval. ready-to-merge This PR has been sent to bors. t-differential-geometry Manifolds etc

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants