Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Merged by Bors] - feat(analysis/inner_product_space/basic): orthogonal submodules #18705

Closed
wants to merge 25 commits into from

Conversation

eric-wieser
Copy link
Member

@eric-wieser eric-wieser commented Mar 30, 2023

This adds submodule.is_ortho U V, with notation U ⟂ V, as a shorthand for U ≤ Vᗮ.
To make this useful, this also adds about 30 lemmas of basic API.

Some downstream proofs are golfed using the new API.


Open in Gitpod

This acts upon a discussion in #18230.

I've tried to add all the obvious API, but there are almost certainly some useful results I've missed. I am happy to add more to this PR.

@eric-wieser eric-wieser added awaiting-review The author would like community review of the PR awaiting-CI The author would like to see what CI has to say before doing more work. labels Mar 30, 2023
@mathlib-dependent-issues-bot mathlib-dependent-issues-bot added the blocked-by-other-PR This PR depends on another PR which is still in the queue. A bot manages this label via PR comment. label Mar 30, 2023
@eric-wieser eric-wieser changed the base branch from master to eric-wieser/split-inner-basic March 30, 2023 15:00
@github-actions github-actions bot removed the awaiting-CI The author would like to see what CI has to say before doing more work. label Mar 30, 2023
@bors bors bot changed the base branch from eric-wieser/split-inner-basic to master March 30, 2023 18:52
@mathlib-dependent-issues-bot mathlib-dependent-issues-bot removed the blocked-by-other-PR This PR depends on another PR which is still in the queue. A bot manages this label via PR comment. label Mar 30, 2023
@mathlib-dependent-issues-bot
Copy link
Collaborator

@eric-wieser eric-wieser requested a review from jsm28 March 30, 2023 19:30
@eric-wieser eric-wieser added the t-analysis Analysis (normed *, calculus) label Mar 31, 2023
Copy link
Collaborator

@YaelDillies YaelDillies left a comment

Choose a reason for hiding this comment

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

I can see this API being very useful, thanks. I saw you had plans for splitting. What are they? Did they already happen?

src/analysis/inner_product_space/gram_schmidt_ortho.lean Outdated Show resolved Hide resolved
(self_is_ortho_orthogonal U).symm

@[simp]
lemma is_ortho_top_right {U : submodule 𝕜 E} : U ⟂ ⊤ ↔ U = ⊥ :=
Copy link
Collaborator

Choose a reason for hiding this comment

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

Okay, this reads funny!

Copy link
Member Author

Choose a reason for hiding this comment

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

I think I derived this name from disjoint_top_right, though I might be imagining things

Copy link
Member

Choose a reason for hiding this comment

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

This is even worse: lemma is_ortho_bot_left {V : submodule 𝕜 E} : ⊥ ⟂ V. Is there any reason why we use (the submodule, not the orthogonality sign) instead of 0?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, we more often care about its order properties than its algebraic ones (which only come in handy when doing pointwise operations).

Copy link
Member Author

Choose a reason for hiding this comment

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

If these are really objectionable I could either write is_ortho ⊥ V or (⊥ : submodule 𝕜 E) ⟂ V here

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it's alright. My remark was inconsequential.

src/analysis/inner_product_space/orthogonal.lean Outdated Show resolved Hide resolved
src/analysis/inner_product_space/orthogonal.lean Outdated Show resolved Hide resolved
src/analysis/inner_product_space/orthogonal.lean Outdated Show resolved Hide resolved
@eric-wieser
Copy link
Member Author

I can see this API being very useful, thanks. I saw you had plans for splitting. What are they? Did they already happen?

The splitting plan already happened, this all used to be in inner_product_space/basic

eric-wieser and others added 3 commits April 3, 2023 09:07
Co-authored-by: Yaël Dillies <yael.dillies@gmail.com>
@@ -1885,6 +1885,7 @@ open_locale direct_sum
The simple way to express this concept would be as a condition on `V : ι → submodule 𝕜 E`. We
We instead implement it as a condition on a family of inner product spaces each equipped with an
isometric embedding into `E`, thus making it a property of morphisms rather than subobjects.
The connection to the subobject spelling is shown in `orthogonal_family_iff_pairwise`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh hmm, if you've already split out orthogonality, why is this still here?

Copy link
Member Author

Choose a reason for hiding this comment

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

I left behind the orthogonal family stuff since it's not about submodules at all.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not opposed to a further split / reshuffle, but I think it's out of scope for this PR.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah I see. The file names are currently misleading.

Copy link
Collaborator

@YaelDillies YaelDillies left a comment

Choose a reason for hiding this comment

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

Glad you took this on.

maintainer merge

@github-actions
Copy link

github-actions bot commented Apr 3, 2023

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

1 similar comment
@github-actions
Copy link

github-actions bot commented Apr 3, 2023

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

@dupuisf
Copy link
Collaborator

dupuisf commented Apr 4, 2023

Thanks!

bors r+

@github-actions github-actions bot added ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.) and removed awaiting-review The author would like community review of the PR labels Apr 4, 2023
bors bot pushed a commit that referenced this pull request Apr 4, 2023
This adds `submodule.is_ortho U V`, with notation `U ⟂ V`, as a shorthand for `U ≤ Vᗮ`.
To make this useful, this also adds about 30 lemmas of basic API.

Some downstream proofs are golfed using the new API.
@bors
Copy link

bors bot commented Apr 4, 2023

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title feat(analysis/inner_product_space/basic): orthogonal submodules [Merged by Bors] - feat(analysis/inner_product_space/basic): orthogonal submodules Apr 4, 2023
@bors bors bot closed this Apr 4, 2023
@bors bors bot deleted the submodule.is_ortho branch April 4, 2023 18:09
xroblot pushed a commit that referenced this pull request Apr 23, 2023
This adds `submodule.is_ortho U V`, with notation `U ⟂ V`, as a shorthand for `U ≤ Vᗮ`.
To make this useful, this also adds about 30 lemmas of basic API.

Some downstream proofs are golfed using the new API.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.) t-analysis Analysis (normed *, calculus)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants