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] - feat(analysis/convex/cone/basic): add has_add, add_zero_class, and add_comm_semigroup instances to convex_cone #16213

Closed
wants to merge 38 commits into from

Conversation

apurvnakade
Copy link
Collaborator

@apurvnakade apurvnakade commented Aug 23, 2022

Adds has_add, add_zero_class, and add_comm_semigroup instance to convex_cones.


#15637 also adds a few other theorems not relevant for this PR.

Open in Gitpod

@apurvnakade apurvnakade changed the title define the sum of two convex cones as a convex cone feat(analysis/convex/cone): define the sum of two convex cones as a convex cone Aug 23, 2022
@apurvnakade apurvnakade added awaiting-review The author would like community review of the PR t-analysis Analysis (normed *, calculus) labels Aug 23, 2022
@mcdoll
Copy link
Member

mcdoll commented Aug 23, 2022

I would guess that your addition and the zero element are compatible in the sense of add_zero_class and that the addition is associative add_semigroup

I haven't tested it, but I would hope that you can use set_like.coe_injective and function.injective.add_semigroup and set.add_semigroup for that.

@apurvnakade
Copy link
Collaborator Author

I would guess that your addition and the zero element are compatible in the sense of add_zero_class and that the addition is associative add_semigroup

I haven't tested it, but I would hope that you can use set_like.coe_injective and function.injective.add_semigroup and set.add_semigroup for that.

Great, thanks! I'll upgrade the classes.

@apurvnakade apurvnakade removed the awaiting-review The author would like community review of the PR label Aug 23, 2022
@apurvnakade apurvnakade 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 Aug 25, 2022
@mathlib-dependent-issues-bot
Copy link
Collaborator

@apurvnakade
Copy link
Collaborator Author

apurvnakade commented Aug 25, 2022

I removed @[simp] lemma coe_add {K₁ K₂ : convex_cone 𝕜 E} : ((K₁ + K₂) : set E) = ↑K₁ + ↑K₂ := rfl because of the following linter error:

/- The syn_taut linter reports: -/
/- THE FOLLOWING DECLARATIONS ARE SYNTACTIC TAUTOLOGIES. This usually means that they are of the form ∀ a b ... z, e₁ = e₂ where e₁ and e₂ are identical expressions. We call declarations of this form syntactic tautologies. Such lemmas are (mostly) useless and sometimes introduced unintentionally when proving basic facts using rfl, when elaboration results in a different term than the user intended. You should check that the declaration really says what you think it does. -/
-- analysis/convex/cone.lean
#check @convex_cone.coe_add /- LHS equals RHS syntactically -/
Error: convex_cone.coe_add - LHS equals RHS syntactically

@apurvnakade apurvnakade changed the title feat(analysis/convex/cone): add has_add, add_zero_class, and add_comm_semigroup instances to convex_cone feat(analysis/convex/cone/basic): add has_add, add_zero_class, and add_comm_semigroup instances to convex_cone Aug 31, 2022
Copy link
Member

@mcdoll mcdoll left a comment

Choose a reason for hiding this comment

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

Sorry for the delay. Looks good to me, even though I am slightly sad that K1 + K2 did not work.

src/analysis/convex/cone/basic.lean Outdated Show resolved Hide resolved
Co-authored-by: mcdoll <moritz.doll@googlemail.com>
@apurvnakade
Copy link
Collaborator Author

apurvnakade commented Sep 22, 2022

Sorry for not the delay. Looks good to me, even though I am slightly sad that K1 + K2 did not work.

No worries! I have also been super busy with teaching so this delay kinda worked out for me.
I'm a bit surprised by the set.has_add issue as well. But I think the API for set.has_add isn't that powerful so not much is lost by not using it.

@mcdoll
Copy link
Member

mcdoll commented Oct 3, 2022

Thanks 🐙

maintainer merge

@github-actions
Copy link

github-actions bot commented Oct 3, 2022

🚀 Pull request has been placed on the maintainer queue by mcdoll.

@hrmacbeth
Copy link
Member

hrmacbeth commented Oct 3, 2022

Thanks!

On the proposed lemma

@[simp] lemma coe_add {K₁ K₂ : convex_cone 𝕜 E} : ((K₁ + K₂) : set E) = ↑K₁ + ↑K₂ := rfl

I might guess the way to have Lean interpret it in a non-tautological way is (↑(K₁ + K₂) : set E) = ↑K₁ + ↑K₂; worth a try later since I agree this is a useful connection.

bors r+

@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 3, 2022
bors bot pushed a commit that referenced this pull request Oct 3, 2022
…d `add_comm_semigroup` instances to `convex_cone` (#16213)

Adds `has_add`, `add_zero_class`, and `add_comm_semigroup` instance to `convex_cone`s.
@bors
Copy link

bors bot commented Oct 3, 2022

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title feat(analysis/convex/cone/basic): add has_add, add_zero_class, and add_comm_semigroup instances to convex_cone [Merged by Bors] - feat(analysis/convex/cone/basic): add has_add, add_zero_class, and add_comm_semigroup instances to convex_cone Oct 3, 2022
@bors bors bot closed this Oct 3, 2022
@bors bors bot deleted the apurva/cone-has-add branch October 3, 2022 03:32
@apurvnakade
Copy link
Collaborator Author

Thanks!

On the proposed lemma

@[simp] lemma coe_add {K₁ K₂ : convex_cone 𝕜 E} : ((K₁ + K₂) : set E) = ↑K₁ + ↑K₂ := rfl

I might guess the way to have Lean interpret it in a non-tautological way is (↑(K₁ + K₂) : set E) = ↑K₁ + ↑K₂; worth a try later since I agree this is a useful connection.

bors r+

I'll try this out and if it works make a small PR. Thank you,

@apurvnakade
Copy link
Collaborator Author

Nope, still the same linter error :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.) t-analysis Analysis (normed *, calculus)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants