Skip to content
This repository was archived by the owner on Jul 24, 2024. It is now read-only.

Conversation

@dupuisf
Copy link
Collaborator

@dupuisf dupuisf commented Sep 7, 2020


This PR redefines the inner product to be based on the is_R_or_C typeclass. Its API is very close to the original one, and whenever a lemma has a cleaner statement in the real case (i.e. if it contains things like re or conj), there is a specialized lemma for that case. The plan would be to then replace real_inner_product.lean by this (which I haven't done yet here). I have ported the rest of mathlib to this new inner product and deleted real_inner_product.lean.

A few things worth noting:

  • The naming scheme: I would suggest replacing names like inner_smul_left by inner.smul_left, with the real and complex cases as inner.real.smul_left and inner.complex.smul_left. I have now switched to just keeping the same naming scheme, with inner replaced with real_inner in a lemma name whenever the lemma only applies to the real inner product, or else tagging _real at the end of the name if inner doesn't appear in the lemma name.
  • The linter complains about a dangerous instance for to_normed_group, because one gets a free parameter. It seems to be local only to this file though, unless I'm misinterpreting what local attribute means.
  • The copyright notice: I just took the notice from real_inner_product and added myself in the list of authors, since a large part of it was copy/pasted from there. If this is inappropriate, please let me know.

@dupuisf dupuisf added the WIP Work in progress label Sep 7, 2020
@github-actions github-actions bot added the merge-conflict Please `git merge origin/master` then a bot will remove this label. label Sep 10, 2020
@github-actions github-actions bot removed the merge-conflict Please `git merge origin/master` then a bot will remove this label. label Sep 10, 2020
@dupuisf dupuisf added awaiting-review The author would like community review of the PR and removed WIP Work in progress labels Sep 11, 2020
Copy link
Collaborator

@sgouezel sgouezel left a comment

Choose a reason for hiding this comment

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

This looks very good to me. My only real question is about the projection part: my impression is that it only needs to be done over the reals, and then it should automatically become available over the complexes thanks to suitable instances.

variables {𝕜 : Type*} [is_R_or_C 𝕜]
local notation `𝓚` := @is_R_or_C.of_real 𝕜 _

/-- Syntactic typeclass for types endowed with an inner product -/
Copy link
Collaborator

Choose a reason for hiding this comment

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

There is a difficulty here that, when you write inner x y, Lean can not guess the field. If you have a (complex) hermitian vector field, and you derive the real inner product space structure, then you will have two different inner spaces on it and Lean will get confused. You have a nice solution below, by introducing the notations ⟪x, y⟫_ℝ and ⟪x, y⟫_ℂ. I think it is worth moving these notations just after the definition and making them global. Also, introducing localized notations for ⟪x, y⟫ as you have done at the end of the file is very good, but you could move them right here. And you could also introduce a third locale inner_product_space where the field is unspecified, and left to Lean. Then the user could choose which locale he wants to open depending on the context. (In this file, you would want to open inner_product_space, for instance) (Maybe you will need to declare the notations in a section, to avoid them being open automatically in the whole file)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I made the ⟪x, y⟫_ℝ and ⟪x, y⟫_ℂ declarations global and moved the rest of the notations up here, but for the inner_product_space localized notation, I suspect that if this is needed elsewhere, one would want to do what I did here: first declare 𝕜 to be some is_R_or_C field, and then declare ⟪x, y⟫ to be the inner product with this specific 𝕜 as the field. I had tried leaving it unspecified and it caused problems.

/- Normally we would want `[is_scalar_tower ℝ 𝕜 α]` in applications as well, to ensure that this
`[normed_space ℝ α]` instance is compatible with the `[normed_space 𝕜 α]` defined above, but
here this is not strictly needed as these lemmas never use the 𝕜 instance. -/
theorem exists_norm_eq_infi_of_complete_convex [normed_space ℝ α] {K : set α} (ne : K.nonempty) (h₁ : is_complete K)
Copy link
Collaborator

Choose a reason for hiding this comment

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

You shouldn't need normed_space ℝ α here. Instead, you should have an instance from normed_space k α to normed_space ℝ α. Or, better, just formulate the theorem for real vector spaces, and let restrict_scalars automatically handle the complex case for you (you would need an instance that a complex inner product space is also a real inner product space, which is good to have in any case)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I actually ran into annoying problems when trying to define these kinds of instances (where is_R_or_C implies real). If I try to declare, for example, that semimodule 𝕜 α implies semimodule ℝ α, Lean complains about the case where 𝕜 = ℝ, because then this new instance is not defeq to the original one. I have no idea how to solve this -- if you have any ideas that would be great!

Copy link
Collaborator

Choose a reason for hiding this comment

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

What about just having an instance that inner_product_space C alpha implies inner_product_space R alpha? And stating all the projection theorems only in real inner product spaces. Then they would also become automatically available in complex inner product spaces as well (since they are just a statement about the norm), which should be enough for applications.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I added the instance (it certainly can't hurt!), but I think it would still be good to prove these for the is_R_or_C case directly, because once we split into two cases, we're condemned to also split everything that gets built on top...

Copy link
Collaborator

@sgouezel sgouezel Sep 29, 2020

Choose a reason for hiding this comment

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

I don't agree with you here: this projection business is specifically a real thing (it requires convexity of the set, which only makes sense in a real context), so it should only be stated for real vector spaces.

The version that would make sense for any is_R_or_C field 𝕜 is that, for any 𝕜-subspace, there is a unique projection on 𝕜 minimizing the distance (which can be stated using only the distance, so not mentioning reals at all). And if you can find properties on the scalar product that only mention 𝕜, then they are worth stating in this context.

So, my suggestion is:

  • Do all the projection business with convex subsets, and everything that mentions , in a specific section that does not mention any field 𝕜
  • Then deduce the results that make sense over any is_R_or_C field (and that don't mention at all) in another section, by registering locally an instance that the space is also a real inner product space, and using the previous results for real inner product spaces. This instance would be a bad instance as you mention (as for 𝕜 = ℝ is creates non-defeq instances) but if it is just a local instance specific to these proofs this is not an issue.

If you don't see how to do this, I can have a go at it if you like.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah I see -- alright I'll give it a shot!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This isn't going too well. That instance does really strange things, even locally (abel stops working in random places, the typeclass system goes nuts on things like semiring ℝ in some of these statements, etc). Now I just rewrote that whole section to just be for real inner products and left the instance unused -- feel free to have a shot at it though.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The trick is to not register this as an instance (even not a local one), but only to use it in proofs to declare explicitly the real structure on a given space, with letI. This means that Lean will not try the instance on this real structure, so nothing bad will happen.

I have done this, and everything went pretty well! Now, the only thing that is specific to real spaces is the projection on convex sets. Orthogonal projection on complete subspaces is done in the general setting of an is_R_or_C field (and I had nothing to change to the proofs, so it definitely convinced me that this is a really efficient way to do things at the same time on reals and complexes!)

It looks ready for merging to me, but before I can merge it someone should have a look to the changes I have made in my last commit. Can you check that you're happy with what I've done?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm certainly happy with it for what it's worth -- thanks a lot!

dupuisf and others added 6 commits September 27, 2020 21:40
Co-authored-by: sgouezel <sebastien.gouezel@univ-rennes1.fr>
Co-authored-by: sgouezel <sebastien.gouezel@univ-rennes1.fr>
Co-authored-by: sgouezel <sebastien.gouezel@univ-rennes1.fr>
@sgouezel sgouezel added awaiting-author A reviewer has asked the author a question or requested changes and removed awaiting-review The author would like community review of the PR labels Sep 28, 2020
@dupuisf dupuisf added awaiting-review The author would like community review of the PR and removed awaiting-author A reviewer has asked the author a question or requested changes labels Sep 29, 2020
@github-actions github-actions bot added the merge-conflict Please `git merge origin/master` then a bot will remove this label. label Sep 30, 2020
@kim-em
Copy link
Collaborator

kim-em commented Sep 30, 2020

This looks really good! I'm happy that the is_R_or_C typeclass is working out.

I'm going to mark this again as awaiting-author so @dupuisf can review @sgouezel's last changes, but I think this is getting very close to merging now.

@github-actions github-actions bot removed the merge-conflict Please `git merge origin/master` then a bot will remove this label. label Oct 1, 2020
@sgouezel
Copy link
Collaborator

sgouezel commented Oct 1, 2020

bors r+
Thanks!

@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 Oct 1, 2020
bors bot pushed a commit that referenced this pull request Oct 1, 2020
@bors
Copy link

bors bot commented Oct 1, 2020

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title feat(analysis/normed_space/inner_product): Define the inner product based on is_R_or_C [Merged by Bors] - feat(analysis/normed_space/inner_product): Define the inner product based on is_R_or_C Oct 1, 2020
@bors bors bot closed this Oct 1, 2020
@bors bors bot deleted the R_or_C_inner_product branch October 1, 2020 09:16
adomani pushed a commit that referenced this pull request Oct 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants