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

feat: IsROrC Rank Lemmas without PartialOrder and StarOrderedRing #6239

Closed
wants to merge 1 commit into from

Conversation

MohanadAhmed
Copy link
Collaborator

This PR proves three lemmas rank_conjTranspose_R_or_C, rank_self_mul_conjTranspose_R_or_C and rank_conjTranspose_mul_self_R_or_C which state that in $\mathbb{R}$ or $\mathbb{C}$ fields:

  • $\text{rank}(A^H) = \text{rank}(A)$
  • $\text{rank}(A^HA) = \text{rank}(A)$
  • $\text{rank}(AA^H) = \text{rank}(A)$

These are almost the same named lemmas (without R_or_C suffix). However the original lemmas don't apply without using a PartialOrder and StarOrderedRing on the Complex Field and IsROrC, which seems to be a point the community is still discussing. The lemmas in this PR apply directly WITHOUT requiring PartialOrder and StarOrderedRing.

Co-authored-by: Mohanad Ahmed m.a.m.elhassan@gmail.com


Open in Gitpod

@semorrison semorrison added the t-analysis Analysis (normed *, calculus) label Aug 6, 2023
@semorrison
Copy link
Contributor

Please change the PR title, this should start with feat:.

@MohanadAhmed MohanadAhmed changed the title IsROrC Rank Lemmas without PartialOrder and StarOrderedRing feat: IsROrC Rank Lemmas without PartialOrder and StarOrderedRing Aug 7, 2023
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.

I don't think these results are necessary. #6391 will be merged soon, and then these results will be available behind the ComplexOrder scope.

@@ -8,6 +8,7 @@ import Mathlib.LinearAlgebra.Matrix.ToLin
import Mathlib.LinearAlgebra.FiniteDimensional
import Mathlib.LinearAlgebra.Matrix.DotProduct
import Mathlib.Data.Complex.Module
import Mathlib.Data.IsROrC.Basic
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is too heavy an import for this file. If you need it, it means the results don't belong here.

@@ -6,6 +6,7 @@ Authors: Johannes Hölzl, Patrick Massot, Casper Putz, Anne Baanen
import Mathlib.Algebra.Star.Order
import Mathlib.Data.Matrix.Basic
import Mathlib.LinearAlgebra.StdBasis
import Mathlib.Data.IsROrC.Basic
Copy link
Collaborator

Choose a reason for hiding this comment

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

again, too heavy an import

Comment on lines +294 to +296
@[simp, isROrC_simps]
theorem re_sum (f : n → K) : IsROrC.re (∑ i in s, f i) = ∑ i in s, IsROrC.re (f i) := by
apply map_sum _ _
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think this is necessary since it is just map_sum IsROrC.reLm _.

@j-loreaux j-loreaux added awaiting-author A reviewer has asked the author a question or requested changes and removed awaiting-review labels Aug 8, 2023
@MohanadAhmed
Copy link
Collaborator Author

I don't think these results are necessary. #6391 will be merged soon, and then these results will be available behind the ComplexOrder scope.

I see.
I think I understand the first point about results will be available through scoped starOrderedRing. But I did not understand the second point about too heavy import? How do you decide if an import is too heavy or not?

Anyway I think I will close this PR for now.

@j-loreaux
Copy link
Collaborator

Whether or not an import is "too heavy" or not is a bit subjective. However, the basic question is: are you importing (even transitively) a lot more than was imported previously? If so, is it essential to the file, or only a few declarations? If the latter, then probably it's too heavy. If it is essential for a lot of new material you are adding, maybe it deserves its own new file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-author A reviewer has asked the author a question or requested changes t-analysis Analysis (normed *, calculus)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants