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.

Sign up for GitHub

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

Jump to bottom

[Merged by Bors] - chore(analysis/inner_product_space/basic): remove bilin_form_of_real_inner #15780

Closed
wants to merge 6 commits into from
Closed

[Merged by Bors] - chore(analysis/inner_product_space/basic): remove bilin_form_of_real_inner #15780

wants to merge 6 commits into from

Conversation

mcdoll
Copy link
Member

@mcdoll mcdoll commented Jul 31, 2022 •
edited by sgouezel

Remove bilin_form_of_real_inner and add a remark in the docstring of sesq_form_of_inner that for over real spaces the sesquilinear form is by definition a bilinear form.

For this we generalize is_self_adjoint_iff_bilin_form from real to is_R_or_C and slightly generalize linear_map.is_self_adjoint and linear_map.is_adjoint_pair to allow for conjugate linear maps in the second argument.

This should be rather easy to review. Note that some of the definitions could be generalized even further, but the point was just to generalize enough to remove the bilin_form definition.

Open in Gitpod

@mcdoll mcdoll added awaiting-CI The author would like to see what CI has to say before doing more work. t-analysis Analysis (normed *, calculus) labels Jul 31, 2022
@mcdoll mcdoll added the awaiting-review The author would like community review of the PR label Jul 31, 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 31, 2022
eric-wieser
eric-wieser reviewed Aug 7, 2022
View reviewed changes
src/analysis/inner_product_space/basic.lean
/-- The inner product as a sesquilinear form. -/
/-- The inner product as a sesquilinear form.

Note that in the case `𝕜 = ℝ` this is a bilinear form. -/
Copy link
Member

Choose a reason for hiding this comment

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

Can you include the recipe to obtain a bilin_form from this?

Copy link
Member

Choose a reason for hiding this comment

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

Right now bilin_form is quite heavily linked to quadratic_form, so we can't burn all the bridges to bilin_form just yet.

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 would not be against having this PR open for a bit until I have removed the dependency of bilin_form in quadratic_form. I was looking at that as well, but I thought this one was super easy to do.

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've added a dependence, so that it does not sit in the review queue

@mathlib-dependent-issues-bot mathlib-dependent-issues-bot added the blocked-by-other-PR This PR depends on another PR which is still in the queue. A bot manages this label via PR comment. label Aug 7, 2022
@mcdoll mcdoll mentioned this pull request Aug 7, 2022
Open
8 tasks
@mathlib-dependent-issues-bot mathlib-dependent-issues-bot removed the blocked-by-other-PR This PR depends on another PR which is still in the queue. A bot manages this label via PR comment. label Oct 4, 2022
@mathlib-dependent-issues-bot
Copy link
Collaborator

This PR/issue depends on:

@sgouezel
Copy link
Collaborator

Can you merge master and fix the build?
bors d+
Thanks!

@bors
Copy link

bors bot commented Oct 10, 2022

✌️ mcdoll 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 delegated The PR author may merge after reviewing final suggestions. and removed awaiting-review The author would like community review of the PR labels Oct 10, 2022
@mcdoll
Copy link
Member Author

mcdoll commented Oct 26, 2022

bors merge

bors bot pushed a commit that referenced this pull request Oct 26, 2022
@mcdoll
chore(analysis/inner_product_space/basic): remove `bilin_form_of_real…
ec3c1f2 
…_inner` (#15780)

Remove `bilin_form_of_real_inner` and add a remark in the docstring of `sesq_form_of_inner` that for over real spaces the sesquilinear form is by definition a bilinear form.

For this we generalize `is_self_adjoint_iff_bilin_form` from `real` to `is_R_or_C` and slightly generalize `linear_map.is_self_adjoint` and `linear_map.is_adjoint_pair` to allow for conjugate linear maps in the second argument.
@bors
Copy link

bors bot commented Oct 26, 2022

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title chore(analysis/inner_product_space/basic): remove bilin_form_of_real_inner [Merged by Bors] - chore(analysis/inner_product_space/basic): remove bilin_form_of_real_inner Oct 26, 2022
@bors bors bot closed this Oct 26, 2022
@bors bors bot deleted the mcdoll/bilinear_refactor branch October 26, 2022 07:42
@eric-wieser
Copy link
Member

I was using bilin_form_of_real_inner downstream; what should I be replacing it with?

eric-wieser added a commit that referenced this pull request Jan 8, 2023
@eric-wieser
fix(analysis/inner_product_space/basic): restore `bilin_form_of_real_…
38b8efe 
…inner`

I was using this downstream as `bilin_form_of_real_inner.to_quadratic_form`, and I don't see a clear replacement.

Until we have api connecting sesquilinear forms with quadratic forms, we should not remove `bilin_form` API.

This partially reverts #15780.
@eric-wieser eric-wieser mentioned this pull request Jan 8, 2023
Closed
bors bot pushed a commit that referenced this pull request Jan 17, 2023
@eric-wieser
fix(analysis/inner_product_space/basic): restore `bilin_form_of_real_…
b4f03bc 
…inner` (#18093)

I was using this downstream as `bilin_form_of_real_inner.to_quadratic_form`, and I don't see a clear replacement.

Until we have api connecting sesquilinear forms with quadratic forms, we should not remove `bilin_form` API.

This partially reverts #15780.
@mcdoll mcdoll mentioned this pull request May 22, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eric-wieser eric-wieser eric-wieser left review comments

Assignees
No one assigned
Labels
delegated The PR author may merge after reviewing final suggestions. t-analysis Analysis (normed *, calculus)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@mcdoll @mathlib-dependent-issues-bot @sgouezel @eric-wieser