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] - refactor(analysis/inner_product_space): rename is_self_adjoint to is_symmetric and add is_self_adjoint #15326

Closed
wants to merge 38 commits into from

Conversation

mcdoll
Copy link
Member

@mcdoll mcdoll commented Jul 13, 2022

We rename the current inner_product_space.is_self_adjoint to linear_map.is_symmetric (which states that inner (A x) y = inner x (A y) for all x,y : E) and add a new definition is_self_adjoint for has_star R. This definition is used to state theorems that were previously stated for linear_map.is_symmetric, but are actually about self-adjointness for continuous_linear_map.
The Hellinger-Toeplitz theorem then becomes the construction of a self-adjoint operator from a symmetric operator, which is consistent with the functional analysis literature.
Moreover, since the definitions are now in the correct namespaces, we can use dot-notation. Consequently, most parts of inner_product_space/rayleigh and inner_product_space/spectrum now use is_self_adjoint and are also now in the continuous_linear_map.is_self_adjoint namespace. For the finite-dimensional case we use is_symmetric, since continuity is not used anywhere.
Finally, there are some minor cleanups in the matrix diagonalization file.


Open in Gitpod

@mcdoll mcdoll added the awaiting-CI The author would like to see what CI has to say before doing more work. label Jul 13, 2022
@vihdzp
Copy link
Collaborator

vihdzp commented Jul 13, 2022

Am I correct in saying an operator is self-adjoint iff it's symmetric, unconditionally? If so, there's definitely no need for two separate predicates.

@mcdoll
Copy link
Member Author

mcdoll commented Jul 14, 2022

The assertion is true modulo distinguishing between continuous_linear_map and linear_map and when your linear_maps are not a special kind of linear_pmaps (which is an implementation detail). The point why mathematicians distinguish between symmetrical and self-adjoint is that the two definitions are not equal for linear_pmaps - we don't have the definition of an adjoint there (yet. I am working on it) - and you cannot call the symmetry condition is_self_adjoint and similarly you cannot say that there is a spectral theorem of symmetric operators, but for self-adjoint operators. So while from a formal point of view you can remove one definition you really can't from a mathematical point of view.

@mcdoll mcdoll added the WIP Work in progress label Jul 14, 2022
@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 Jul 17, 2022
@mcdoll mcdoll added awaiting-review The author would like community review of the PR and removed WIP Work in progress labels Jul 17, 2022
Copy link
Collaborator

@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.

Other than some small golfs I think this looks good.

src/analysis/inner_product_space/adjoint.lean Outdated Show resolved Hide resolved
src/analysis/inner_product_space/adjoint.lean Outdated Show resolved Hide resolved
src/analysis/inner_product_space/adjoint.lean Outdated Show resolved Hide resolved
src/analysis/inner_product_space/adjoint.lean Outdated Show resolved Hide resolved
mcdoll and others added 2 commits July 23, 2022 10:10
Co-authored-by: Jireh Loreaux <loreaujy@gmail.com>
Co-authored-by: Jireh Loreaux <loreaujy@gmail.com>
@mcdoll mcdoll added the t-analysis Analysis (normed *, calculus) label Jul 24, 2022
@github-actions github-actions bot added the delegated The PR author may merge after reviewing final suggestions. label Jul 27, 2022
Copy link
Collaborator

@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.

There are a few more minor things to do, but it's getting close. Moreover, I think this diff makes it clear that this (the is_self_adjoint predicate) is a good change.

src/algebra/star/self_adjoint.lean Outdated Show resolved Hide resolved
src/algebra/star/self_adjoint.lean Outdated Show resolved Hide resolved
src/algebra/star/self_adjoint.lean Outdated Show resolved Hide resolved
src/algebra/star/self_adjoint.lean Show resolved Hide resolved
src/algebra/star/self_adjoint.lean Outdated Show resolved Hide resolved
src/algebra/star/self_adjoint.lean Outdated Show resolved Hide resolved
src/analysis/inner_product_space/adjoint.lean Outdated Show resolved Hide resolved
src/algebra/star/self_adjoint.lean Outdated Show resolved Hide resolved
mcdoll and others added 6 commits August 4, 2022 01:33
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>
local notation `⟪`x`, `y`⟫` := @inner 𝕜 _ _ x y

/-- A continuous linear endomorphism `T` of a Hilbert space is **positive** if it is self adjoint
and `∀ x, 0 ≤ re ⟪T x, x⟫`. -/
def is_positive (T : E →L[𝕜] E) : Prop :=
is_self_adjoint (T : E →ₗ[𝕜] E) ∧ ∀ x, 0 ≤ T.re_apply_inner_self x
is_self_adjoint T ∧ ∀ x, 0 ≤ T.re_apply_inner_self x
Copy link
Member

Choose a reason for hiding this comment

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

Is this a regression? This seems to me like a situation where it would be convenient to keep using is_symmetric rather than switch to the new is_self_adjoint. The current definition requires E →L[𝕜] E but the early lemmas will work for E →ₗ[𝕜] E and it seems like we should eventually switch to that setting. Unnecessary dependence on the adjoint here will make that impossible.

(If I missed an earlier discussion of this point, feel free to point me to it.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Of course the lemmas work for both cases, they are equivalent. My point was if the operator is a-priori defined as a continuous linear operator, then one should use is_self_adjoint and if it is defined as a linear operator, then is_symmetric is the correct definition (as there is in general (i.e., infinite dimension) no adjoint).

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 I agree with Heather here. This is the concrete definition of positivity, so the included selfadjoint condition should really be is_symmetric defined in terms of the inner product. Regressions aside, at the very least, if you're proving an operator is positive this way, it will likely be easy to show that it is symmetric (because you apparently have control over the values of the quadratic form), and then you don't need to invoke Hellinger-Toeplitz to get the selfadjoint condition.

Copy link
Member Author

Choose a reason for hiding this comment

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

Note that in the definition we assume that the operator is continuous. We could add a definition that constructs positive operator from linear_map that is symmetric and satisfies ∀ x, 0 ≤ T.re_apply_inner_self x. That would be the easiest way to construct a positive operator. I guess there is literature that proves the boundedness and the symmetry, but this is obviously redundant.

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 maybe we are talking past each other, but I can't quite tell. Why not just have linear_map.is_positive defined as is_symmetric and ∀ x, 0 ≤ T.re_apply_inner_self x? Then it would apply in both situations (admittedly, dot notation wouldn't work so well anymore in the continuous case).

However, if your point is that is_positive should really be defined as is_self_adjoint and ∀ x, 0 ≤ T.re_apply_inner_self x for linear_pmaps in the unbounded case (as opposed to using is_symmetric), then I would prefer any definition which would unify the three settings, although I have some doubts whether this is really possible.

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 agree that we might be not talking about the same thing. The current is_positive is defined for continuous_linear_map, so my point is that there is no reason to prefer is_symmetric over is_self_adjoint.
If it was defined for linear_map, then I would agree that is_symmetric is the correct choice. I don't think that changing the definition of is_positive from continuous_linear_map to linear_map is in the scope of this PR (I think the definition as it is now is good). Also, I don't really see the value in adding a definition linear_map.is_positive, but maybe you have a good reason for that. There might be an argument that it is necessary for the finite dimensional case, because we don't want to talk about continuous_linear_maps in finite dimensions.
As for the linear_pmap I cannot guarantee anything, but I am rather optimistic that we will need only two definitions (we could get away with one, but this would cause lots of annoying things with coercing to linear_pmap).

I am sorry if I come across as too stubborn on this, I only think that reverting to is_symmetric for continuous_linear_map would make this whole PR completely useless.

has_eigenvalue T ↑(⨆ x : {x : E // x ≠ 0}, is_R_or_C.re ⟪T x, x⟫ / ∥(x:E)∥ ^ 2) :=
begin
haveI := finite_dimensional.proper_is_R_or_C 𝕜 E,
let T' : E →L[𝕜] E := T.to_continuous_linear_map,
have hT' : is_self_adjoint (T' : E →ₗ[𝕜] E) := hT,
let T' := hT.to_self_adjoint,
Copy link
Member

Choose a reason for hiding this comment

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

This also seems like a regression to me? This file currently does not "really" depend on the adjoint; it just has a spurious dependence introduced by #15281. But at this line you are proving a fact about a symmetric operator (in finite dimension) by invoking that it's self-adjoint (in the new, adjoint-dependent sense), then at line 245 calling a result is_self_adjoint.has_eigenvector_of_is_max_on which is proved for self-adjoint operators by at some point invoking hT.is_symmetric, the fact that they are symmetric.

After the renaming PR goes through I hope immediately to separate out is_symmetric, and make the Rayleigh file import only that. If that's to be possible, then this file needs to stay more or less as it was.

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 guess you could argue that the finite dimension part should use is_self_adjoint (E →ₗ[𝕜] E) with finite dimensional adjoint, but it felt to me that that was really splitting hairs.

Copy link
Member Author

Choose a reason for hiding this comment

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

In this case here I was just using the totally overblown Hellinger-Toeplitz theorem to upgrade a linear map to a continuous linear map, so that on can use the things that are defined for is_self_adjoint (E →L[𝕜] E)

Copy link
Member

Choose a reason for hiding this comment

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

I guess you could argue that the finite dimension part should use is_self_adjoint (E →ₗ[𝕜] E) with finite dimensional adjoint, but it felt to me that that was really splitting hairs.

I was actually arguing the opposite, that the infinite dimension part should use is_symmetric ...

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 know, but the whole point why I made this refactor is that I find that this goes too much against the mathematical standards. Technically you could say that the spectral theorem holds for bounded symmetric operators, but nobody does that. Moreover, the Rayleigh quotient is very useful for unbounded operators, so I would argue that the correct definition would be for is_self_adjoint (linear_pmap R E F), but unfortunately the adjoint is not yet defined for linear_pmap

@j-loreaux
Copy link
Collaborator

Per the discussion on Zulip, it seems that this PR is okay as-is.
bors r+

bors bot pushed a commit that referenced this pull request Aug 15, 2022
…is_symmetric` and add `is_self_adjoint` (#15326)

We rename the current `inner_product_space.is_self_adjoint` to `linear_map.is_symmetric` (which states that `inner (A x) y = inner x (A y)` for all `x,y : E`) and add a new definition `is_self_adjoint` for `has_star R`. This definition is used to state theorems that were previously stated for `linear_map.is_symmetric`, but are actually about self-adjointness for `continuous_linear_map`. 
The Hellinger-Toeplitz theorem then becomes the construction of a self-adjoint operator from a symmetric operator, which is consistent with the functional analysis literature.
Moreover, since the definitions are now in the correct namespaces, we can use dot-notation. Consequently, most parts of `inner_product_space/rayleigh` and `inner_product_space/spectrum` now use `is_self_adjoint` and are also now in the `continuous_linear_map.is_self_adjoint` namespace. For the finite-dimensional case we use `is_symmetric`, since continuity is not used anywhere.
Finally, there are some minor cleanups in the matrix diagonalization file.



Co-authored-by: Moritz Doll <doll@uni-bremen.de>
@bors
Copy link

bors bot commented Aug 15, 2022

Build failed (retrying...):

bors bot pushed a commit that referenced this pull request Aug 15, 2022
…is_symmetric` and add `is_self_adjoint` (#15326)

We rename the current `inner_product_space.is_self_adjoint` to `linear_map.is_symmetric` (which states that `inner (A x) y = inner x (A y)` for all `x,y : E`) and add a new definition `is_self_adjoint` for `has_star R`. This definition is used to state theorems that were previously stated for `linear_map.is_symmetric`, but are actually about self-adjointness for `continuous_linear_map`. 
The Hellinger-Toeplitz theorem then becomes the construction of a self-adjoint operator from a symmetric operator, which is consistent with the functional analysis literature.
Moreover, since the definitions are now in the correct namespaces, we can use dot-notation. Consequently, most parts of `inner_product_space/rayleigh` and `inner_product_space/spectrum` now use `is_self_adjoint` and are also now in the `continuous_linear_map.is_self_adjoint` namespace. For the finite-dimensional case we use `is_symmetric`, since continuity is not used anywhere.
Finally, there are some minor cleanups in the matrix diagonalization file.



Co-authored-by: Moritz Doll <doll@uni-bremen.de>
@bors
Copy link

bors bot commented Aug 15, 2022

Build failed (retrying...):

@bors
Copy link

bors bot commented Aug 15, 2022

Build failed (retrying...):

@j-loreaux
Copy link
Collaborator

bors r+

bors bot pushed a commit that referenced this pull request Aug 16, 2022
…is_symmetric` and add `is_self_adjoint` (#15326)

We rename the current `inner_product_space.is_self_adjoint` to `linear_map.is_symmetric` (which states that `inner (A x) y = inner x (A y)` for all `x,y : E`) and add a new definition `is_self_adjoint` for `has_star R`. This definition is used to state theorems that were previously stated for `linear_map.is_symmetric`, but are actually about self-adjointness for `continuous_linear_map`. 
The Hellinger-Toeplitz theorem then becomes the construction of a self-adjoint operator from a symmetric operator, which is consistent with the functional analysis literature.
Moreover, since the definitions are now in the correct namespaces, we can use dot-notation. Consequently, most parts of `inner_product_space/rayleigh` and `inner_product_space/spectrum` now use `is_self_adjoint` and are also now in the `continuous_linear_map.is_self_adjoint` namespace. For the finite-dimensional case we use `is_symmetric`, since continuity is not used anywhere.
Finally, there are some minor cleanups in the matrix diagonalization file.



Co-authored-by: Moritz Doll <doll@uni-bremen.de>
@bors
Copy link

bors bot commented Aug 16, 2022

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title refactor(analysis/inner_product_space): rename is_self_adjoint to is_symmetric and add is_self_adjoint [Merged by Bors] - refactor(analysis/inner_product_space): rename is_self_adjoint to is_symmetric and add is_self_adjoint Aug 16, 2022
@bors bors bot closed this Aug 16, 2022
@bors bors bot deleted the mcdoll/symmetric_rename branch August 16, 2022 06:42
bors bot pushed a commit that referenced this pull request Aug 25, 2022
… A` to `is_self_adjoint a` (#16212)

After #15326, this PR migrates existing uses of `a ∈ self_adjoint A` to `is_self_adjoint a` so as to standardize. We also move several results into the `is_self_adjoint` namespace in order to take advantage of dot notation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
delegated The PR author may merge after reviewing final suggestions. t-analysis Analysis (normed *, calculus)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants