Skip to content

[Merged by Bors] - feat(Analysis/InnerProductSpace/Basic): add PreInnerProductSpace#14024

Closed
yoh-tanimoto wants to merge 31 commits intomasterfrom
yoh-tanimoto-preinner
Closed

[Merged by Bors] - feat(Analysis/InnerProductSpace/Basic): add PreInnerProductSpace#14024
yoh-tanimoto wants to merge 31 commits intomasterfrom
yoh-tanimoto-preinner

Conversation

@yoh-tanimoto
Copy link
Copy Markdown
Collaborator

@yoh-tanimoto yoh-tanimoto commented Jun 21, 2024

add the structure PreInnerProductSpace, which does not assume definite for the inner product, with a proof of the Cauchy-Schwarz inequality using the discriminant.

Motivation: Such structures arise very often in applications, and by quotienting the kernel one obtains InnerProductSpace.Core.

I duplicated most of the proofs from the namespace InnerProductSpace.Core and put them under the namespace PreInnerProductSpace . Is there a better way?

@yoh-tanimoto yoh-tanimoto added awaiting-review awaiting-CI This PR does not pass CI yet. This label is automatically removed once it does. t-analysis Analysis (normed *, calculus) new-contributor This PR was made by a contributor with at most 5 merged PRs. Welcome to the community! labels Jun 21, 2024
@github-actions
Copy link
Copy Markdown

github-actions bot commented Jun 21, 2024

PR summary 5f93724fb7

Import changes for modified files

Dependency changes

File Base Count Head Count Change
Mathlib.Analysis.InnerProductSpace.Basic 1470 1471 +1 (+0.07%)
Mathlib.LinearAlgebra.Matrix.PosDef 1679 1680 +1 (+0.06%)
Import changes for all files
Files Import difference
There are 247 files with changed transitive imports: this is too many to display!

Declarations diff

+ PreInnerProductSpace.Core
+ PreInnerProductSpace.toCore
+ cauchy_schwarz_aux'
+ inner_self_of_eq_zero
+ inner_smul_ofReal_left
+ inner_smul_ofReal_right
+ instance (𝕜 : Type*) (F : Type*) [RCLike 𝕜] [AddCommGroup F]
+ instance : InnerProductSpace ℝ ℂ := InnerProductSpace.complexToReal
+ ne_zero_of_inner_self_ne_zero
+ normSq_eq_zero_of_eq_zero
+ re_inner_smul_ofReal_smul_self
+ toPreInner'
+ toSeminormedAddCommGroup
+ toSeminormedSpace

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.

@github-actions github-actions bot removed the awaiting-CI This PR does not pass CI yet. This label is automatically removed once it does. label Jun 21, 2024
@j-loreaux
Copy link
Copy Markdown
Contributor

Well, you should remove the inner product versions for these that are strictly more general, as a first step.

@ADedecker do you have thoughts about this preinner product stuff before we get into the weeds of review?

@ADedecker
Copy link
Copy Markdown
Member

ADedecker commented Jun 21, 2024

I'm sorry I have absolutely no time to help reviewing this, I'm currently buried under PHD-funding paperwork and the related stress... But I did think about this some time ago, so here are my thoughts.

I strongly think the correct approach for the general theory is simply to assume only SeminormedAddCommGroup E in the definition of InnerProductSpace and in all the lemmas that work without definiteness : that way there is only one definition, and the choice to impose definiteness or not is obtained by choosing between SeminormedAddCommGroup and NormedAddCommGroup.

Of course this trick doesn't work for the "Core" version since it doesn't come with a norm. I am not so sure that it's the correct approach, but I would still say to just remove the definiteness from the definition, and add a InnerProductSpace.normedAddCommGroupOfDefinite constructor that also asks for a separate proof of definiteness. But please check it won't be too much of a hassle to use before going down that road.

Anyway, thanks for working on this!

@yoh-tanimoto
Copy link
Copy Markdown
Collaborator Author

@j-loreaux @ADedecker I see, I thought this would be a small PR, but instead there seems to be much to do! I probably need some time to do anything reasonable. Meanwhile, if you have comments on how things should be organized, please do so.

@j-loreaux
Copy link
Copy Markdown
Contributor

I completely agree with Anatole here. I had forgotten we separated out the NormedAddCommGroup so that it is a parameter instead of having InnerProductSpace extend it. So, the sensible thing to do as a first pass on this PR is the following: Replace NormedAddCommGroup with SeminormedAddCommGroup in the definition of InnerProductSpace. See what breaks, then in those places that it breaks, check that definiteness is required, and in those situations add a NormedAddCommGroup assumption (without overlapping the Seminormed one). Does this make sense @yoh-tanimoto ?

@j-loreaux j-loreaux added awaiting-author A reviewer has asked the author a question or requested changes. and removed awaiting-review labels Jun 24, 2024
@yoh-tanimoto
Copy link
Copy Markdown
Collaborator Author

I tried, and the first issue is that, since I remove definite from Core, things break not with NormedAddCommGroup but when definite is used. What should I do with theorems using definite? @j-loreaux

@j-loreaux
Copy link
Copy Markdown
Contributor

You should have two .Core things, one with definiteness and one with semi definiteness. On one you get a NormedAddCommGroup instance, and on the other you get a SeminormedAddCommGroup instance. It shouldn't be too big a deal because these should almost never be used anyway since you don't get good defeqs.for the norm.

@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 24, 2024
@yoh-tanimoto
Copy link
Copy Markdown
Collaborator Author

I made two .Cores. Now the problem is where I want to prove theorems for cd : InnerProductSpace.Core 𝕜 F, but the local notation ⟪x, y⟫ is given for c : PreInnerProductSpace.Core 𝕜 F. How can I update the notation?

@j-loreaux
Copy link
Copy Markdown
Contributor

You can just close the scope for the existing local notation (this might mean moving it into a section), and write a new local notation. However, what probably makes the most sense is to separate out the stuff involving InnerProductSpace.Core into its own section instead of having them intermingled.

@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 26, 2024
@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 Jul 1, 2024
@github-actions github-actions bot removed the awaiting-CI This PR does not pass CI yet. This label is automatically removed once it does. label Aug 6, 2024
Copy link
Copy Markdown
Contributor

@j-loreaux j-loreaux left a comment

Choose a reason for hiding this comment

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

These are the last corrections. Ping me when you're finished and I'll merge.

@j-loreaux j-loreaux added the awaiting-author A reviewer has asked the author a question or requested changes. label Aug 6, 2024
yoh-tanimoto and others added 5 commits August 7, 2024 01:41
Co-authored-by: Jireh Loreaux <loreaujy@gmail.com>
Co-authored-by: Jireh Loreaux <loreaujy@gmail.com>
Co-authored-by: Jireh Loreaux <loreaujy@gmail.com>
Co-authored-by: Jireh Loreaux <loreaujy@gmail.com>
Co-authored-by: Jireh Loreaux <loreaujy@gmail.com>
@yoh-tanimoto yoh-tanimoto added awaiting-CI This PR does not pass CI yet. This label is automatically removed once it does. and removed awaiting-author A reviewer has asked the author a question or requested changes. labels Aug 6, 2024
@github-actions github-actions bot removed the awaiting-CI This PR does not pass CI yet. This label is automatically removed once it does. label Aug 7, 2024
@yoh-tanimoto
Copy link
Copy Markdown
Collaborator Author

@j-loreaux done!

@j-loreaux
Copy link
Copy Markdown
Contributor

Can you please merge master, then wait for CI to finish? After that, assuming CI is successful, you're welcome to merge.

bors d+

@mathlib-bors
Copy link
Copy Markdown
Contributor

mathlib-bors bot commented Aug 7, 2024

✌️ yoh-tanimoto can now approve this pull request. To approve and merge a pull request, simply reply with bors r+. More detailed instructions are available here.

@github-actions github-actions bot added the delegated This pull request has been delegated to the PR author (or occasionally another non-maintainer). label Aug 7, 2024
@yoh-tanimoto
Copy link
Copy Markdown
Collaborator Author

bors r+

mathlib-bors bot pushed a commit that referenced this pull request Aug 7, 2024
)

add the structure `PreInnerProductSpace`, which does not assume `definite` for the inner product, with a proof of the Cauchy-Schwarz inequality using the discriminant.

Motivation: Such structures arise very often in applications, and by quotienting the kernel one obtains `InnerProductSpace.Core`.

I duplicated most of the proofs from the namespace `InnerProductSpace.Core` and put them under the namespace `PreInnerProductSpace` . Is there a better way?
@mathlib-bors
Copy link
Copy Markdown
Contributor

mathlib-bors bot commented Aug 7, 2024

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title feat(Analysis/InnerProductSpace/Basic): add PreInnerProductSpace [Merged by Bors] - feat(Analysis/InnerProductSpace/Basic): add PreInnerProductSpace Aug 7, 2024
@mathlib-bors mathlib-bors bot closed this Aug 7, 2024
@mathlib-bors mathlib-bors bot deleted the yoh-tanimoto-preinner branch August 7, 2024 01:42
bjoernkjoshanssen pushed a commit that referenced this pull request Sep 9, 2024
)

add the structure `PreInnerProductSpace`, which does not assume `definite` for the inner product, with a proof of the Cauchy-Schwarz inequality using the discriminant.

Motivation: Such structures arise very often in applications, and by quotienting the kernel one obtains `InnerProductSpace.Core`.

I duplicated most of the proofs from the namespace `InnerProductSpace.Core` and put them under the namespace `PreInnerProductSpace` . Is there a better way?
bjoernkjoshanssen pushed a commit that referenced this pull request Sep 9, 2024
)

add the structure `PreInnerProductSpace`, which does not assume `definite` for the inner product, with a proof of the Cauchy-Schwarz inequality using the discriminant.

Motivation: Such structures arise very often in applications, and by quotienting the kernel one obtains `InnerProductSpace.Core`.

I duplicated most of the proofs from the namespace `InnerProductSpace.Core` and put them under the namespace `PreInnerProductSpace` . Is there a better way?
bjoernkjoshanssen pushed a commit that referenced this pull request Sep 12, 2024
)

add the structure `PreInnerProductSpace`, which does not assume `definite` for the inner product, with a proof of the Cauchy-Schwarz inequality using the discriminant.

Motivation: Such structures arise very often in applications, and by quotienting the kernel one obtains `InnerProductSpace.Core`.

I duplicated most of the proofs from the namespace `InnerProductSpace.Core` and put them under the namespace `PreInnerProductSpace` . Is there a better way?
mathlib-bors bot pushed a commit that referenced this pull request Sep 25, 2024
…ddCommGroup` (#17007)

replace the assumption `NormedAddCommGroup` to `SemiNormedAddCommGroup` in various places.

motivation: with the weakened assumption the results apply to `InnerProductSpace` without `definite` assumption.

This is suggested in #16707, and continues from #14024.



Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
fbarroero pushed a commit that referenced this pull request Oct 2, 2024
…ddCommGroup` (#17007)

replace the assumption `NormedAddCommGroup` to `SemiNormedAddCommGroup` in various places.

motivation: with the weakened assumption the results apply to `InnerProductSpace` without `definite` assumption.

This is suggested in #16707, and continues from #14024.



Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

delegated This pull request has been delegated to the PR author (or occasionally another non-maintainer). new-contributor This PR was made by a contributor with at most 5 merged PRs. Welcome to the community! t-analysis Analysis (normed *, calculus)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants