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

chore(analysis/inner_product_space): remove two redundant definitions (again) #19058

Closed
wants to merge 6 commits into from

Conversation

mcdoll
Copy link
Member

@mcdoll mcdoll commented May 22, 2023

This PR removes sesq_form_of_inner and bilin_form_of_real_inner. The former one is just (innerₛₗ 𝕜).flip and the later one can be easily replaced by innerₛₗ 𝕜. The definition bilin_form_of_real_inner was earlier removed in #15780, but then restored in #18093. Since then nothing has been added to mathlib that makes bilin_form_of_real_inner necessary. In particular the mentioned quadratic form is just the square of the norm.

We also generalize bilin_form_of_real_inner_orthogonal and the two versions of is_adjoint_pair_inner so that they hold for complex inner products and not just the reals.

Add a few comments for a proof in the Riemann-Lebesgue lemma which is unnecessarily complicated, but fixing this is not in the scope of this PR.


Open in Gitpod

@mcdoll mcdoll added the WIP Work in progress label May 22, 2023
@mcdoll mcdoll 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. t-analysis Analysis (normed *, calculus) and removed WIP Work in progress labels May 22, 2023
@@ -60,10 +60,14 @@ variables [normed_add_comm_group V] [measurable_space V] [borel_space V]
lemma fourier_integrand_integrable (w : V) :
integrable f ↔ integrable (λ v : V, e [-⟪v, w⟫] • f v) :=
begin
have hL : continuous (λ p : V × V, bilin_form_of_real_inner.to_lin p.1 p.2) := continuous_inner,
have hL : continuous (λ p : V × V, innerₛₗ ℝ p.1 p.2) := continuous_inner,
-- TODO: remove the continuity assumption from `vector_fourier.fourier_integral_convergent_iff`
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 the lemma is still true without this assumption, as you can get measurability issues, no?

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 haven't checked the details, but I agree that one probably has to assume measurability.


/-- The real inner product as a bilinear form. -/
@[simps]
def bilin_form_of_real_inner : bilin_form ℝ F :=
Copy link
Member

@eric-wieser eric-wieser May 22, 2023

Choose a reason for hiding this comment

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

I need this definition in a downstream project. What am I supposed to use instead if you're removing it?

(I agree with the removal of sesq_form_of_inner as it has an easy replacement)

Copy link
Member

Choose a reason for hiding this comment

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

@eric-wieser Can you point us to the downstream project?

Copy link
Member

Choose a reason for hiding this comment

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

Sure, I use it here and here.

It's important to me that these terms are of the form bilin_form.to_quadratic_form B, as that's the language used when working with clifford_algebra.change_form_equiv.

In the long run I agree it makes sense to remove this def; but in the short term, the quadratic_form API is very bilin_form-centric, and I don't think we should be deleting bilin_form API until we've finished refactoring quadratic_form.

Copy link
Member Author

Choose a reason for hiding this comment

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

Can't you just add bilin_form_of_real_inner in that project? As you can see if we keep the definition in mathlib, then it gets used in places where we have better things.

Copy link
Member

@eric-wieser eric-wieser May 22, 2023

Choose a reason for hiding this comment

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

Can't you just add bilin_form_of_real_inner in that project?

Yes, but I don't see why I should need to. Until bilin_form is deprecated / made an abbreviation, I don't see what we gain by removing parts of its API.

As you can see if we keep the definition in mathlib, then it gets used in places where we have better things.

I think a better solution would be to either:

  • Edit the docstring to mention innerₛₗ ℝ : F →ₗ⋆[ℝ] F →ₗ[ℝ] ℝ as an alternative to bilin_form_of_real_inner : bilin_form ℝ F
  • Add a simp-lemma that says bilin_form_of_real_inner.to_lin = innerₛₗ ℝ (proof should be ext, refl)

Mathlib is full of non-simp-normal bundlings; the way we indicate that they should not be used unless the specific bundling is needed is by simping them away and adding docstrings.

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 consider bilin_form deprecated. Would you be happy with adding

namespace linear_map

variables [comm_semiring R] [add_comm_monoid M] [module R M]

def to_quadratic_form (B : M →ₗ[R] M →ₗ[R] R) : quadratic_form R M :=
  B.to_bilin.to_quadratic_form

@[simp] lemma to_quadratic_form_apply (B : M →ₗ[R] M →ₗ[R] R) (x : M) :
  B.to_quadratic_form x = B x x :=
rfl

end linear_map

to the quadratic form file?

Copy link
Member

@eric-wieser eric-wieser May 22, 2023

Choose a reason for hiding this comment

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

Those 10 lines aren't a substitute for the 70 lines of API lemmas here, and the handful of API lemmas spread around the file.

Why don't we just leave the definition around until mathlib4, and tag it @[deprecated] there? This file is on the critical porting path now; and I'd strongly prefer the frozen version in mathlib3 not be one that doesn't work in my project.
I don't think there's any urgency to delete it, and the other changes in this PR look good to me.

@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 May 22, 2023
@eric-wieser eric-wieser 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 May 22, 2023

@[simp] lemma innerₛₗ_apply (v w : E) : innerₛₗ 𝕜 v w = ⟪v, w⟫ := rfl

variables {𝕜}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
variables {𝕜}
variables {𝕜}
/-- The real inner product as a bilinear form. -/
@[simps]
def bilin_form_of_real_inner : bilin_form ℝ F :=
{ bilin := inner,
bilin_add_left := inner_add_left,
bilin_smul_left := λ a x y, inner_smul_left _ _ _,
bilin_add_right := inner_add_right,
bilin_smul_right := λ a x y, inner_smul_right _ _ _ }
@[simp]
lemma bilin_form_of_real_inner_to_lin : (bilin_form_of_real_inner : bilin_form ℝ F).to_lin = innerₛₗ ℝ := rfl

as discussed above

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

4 participants