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/strict_convex_space): Strictly convex spaces #11794

Closed
wants to merge 40 commits into from

Conversation

YaelDillies
Copy link
Collaborator

@YaelDillies YaelDillies commented Feb 3, 2022

@YaelDillies YaelDillies added the awaiting-review The author would like community review of the PR label Feb 3, 2022
@urkud
Copy link
Member

urkud commented Feb 9, 2022

I'm sorry for a long silence: I was busy with my ITP paper. I mostly like the code. Tomorrow I'm going to double check that TC assumptions in the new "two semirings" lemmas are indeed minimal. Also, I don't like the new lemmas that represent (closed) balls as images of the unit (closed) ball under some map. I think that we should swap LHS with RHS and name them smul_unit_ball etc.

BTW, could you please prove that the triangle inequality is an equality iff one of the points belongs to the segment (probably, starting with a proof that ∥u + v∥ = ∥u∥ + ∥v∥ implies that u and v are proportional)?

@YaelDillies
Copy link
Collaborator Author

Let's give it a go!

@RemyDegenne RemyDegenne 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 Feb 10, 2022
@leanprover-community-bot-assistant leanprover-community-bot-assistant added the merge-conflict Please `git merge origin/master` then a bot will remove this label. label Feb 15, 2022
@leanprover-community-bot-assistant leanprover-community-bot-assistant removed the merge-conflict Please `git merge origin/master` then a bot will remove this label. label Feb 20, 2022
@leanprover-community-bot-assistant leanprover-community-bot-assistant added the merge-conflict Please `git merge origin/master` then a bot will remove this label. label Feb 23, 2022
@leanprover-community-bot-assistant leanprover-community-bot-assistant added merge-conflict Please `git merge origin/master` then a bot will remove this label. and removed merge-conflict Please `git merge origin/master` then a bot will remove this label. labels Feb 24, 2022
@leanprover-community-bot-assistant leanprover-community-bot-assistant removed the merge-conflict Please `git merge origin/master` then a bot will remove this label. label Mar 2, 2022
@leanprover-community-bot-assistant leanprover-community-bot-assistant 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 Mar 12, 2022
@leanprover-community-bot-assistant leanprover-community-bot-assistant 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 Mar 17, 2022
@leanprover-community-bot-assistant leanprover-community-bot-assistant added the merge-conflict Please `git merge origin/master` then a bot will remove this label. label Mar 17, 2022
@leanprover-community-bot-assistant leanprover-community-bot-assistant removed the merge-conflict Please `git merge origin/master` then a bot will remove this label. label Mar 17, 2022
Copy link
Collaborator

@ericrbg ericrbg left a comment

Choose a reason for hiding this comment

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

lgtm though! i guess the two things I noticed have good reasons for being this way

src/analysis/convex/strict_convex_space.lean Outdated Show resolved Hide resolved
src/analysis/convex/strict_convex_space.lean Show resolved Hide resolved
Co-authored-by: Eric Rodriguez <37984851+ericrbg@users.noreply.github.com>
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.

I don't know anything about strictly convex spaces. The code looks good.

src/analysis/convex/strict_convex_space.lean Show resolved Hide resolved
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.

Mainly I'm not quite sure why you chose this particular definition, see comments within.

Comment on lines +20 to +21
field (e.g., `ℝ` or `ℚ`) is strictly convex. The definition requires strict convexity of a closed
ball of positive radius with center at the origin; strict convexity of any other closed ball follows
Copy link
Collaborator

Choose a reason for hiding this comment

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

I know it follows from the strict convexity of a single ball, but the definition you gave is in terms of all balls of positive radius centered at the origin.

Suggested change
field (e.g., `ℝ` or `ℚ`) is strictly convex. The definition requires strict convexity of a closed
ball of positive radius with center at the origin; strict convexity of any other closed ball follows
field (e.g., `ℝ` or `ℚ`) is strictly convex. The definition requires strict convexity of all closed
balls of positive radius with center at the origin; strict convexity of any other closed ball follows

See also `strict_convex_space.of_strict_convex_closed_unit_ball`. -/
class strict_convex_space (𝕜 E : Type*) [normed_linear_ordered_field 𝕜] [normed_group E]
[normed_space 𝕜 E] : Prop :=
(strict_convex_closed_ball : ∀ r : ℝ, 0 < r → strict_convex 𝕜 (closed_ball (0 : E) r))
Copy link
Collaborator

Choose a reason for hiding this comment

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

I feel like this is a weird middle ground to have as the definition. What I mean is this: strict convexity follows from strict convexity of the unit ball, and in a strictly convex space, all balls are strictly convex. So, in the definition, I would expect one of those two (unit ball or all balls), and presumably the former with a theorem for the latter. As it is now, whenever someone makes a strict_convex_space they will always opt for the constructor lemma strict_convex_space.of_strict_convex_closed_unit_ball, but instead you could just make that the definition. So, to be clear, I'm asking for:

class strict_convex_space (𝕜 E : Type*) [normed_linear_ordered_field 𝕜] [normed_group E]
  [normed_space 𝕜 E] : Prop :=
(strict_convex_closed_ball : strict_convex 𝕜 (closed_ball (0 : E) (1 : ℝ))

With a new proof of strict_convex_closed_ball and deleting strict_convex_space.of_strict_convex_closed_unit_ball. Of course, if you implement this, ignore the documentation comments above about "all balls"

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is the point that strict convexity of the unit ball only implies strict convexity of all other balls if E is an -vector space? If so, is there an easy counterexample? In that case, just say so and ignore this comment.

Copy link
Member

@urkud urkud Mar 25, 2022

Choose a reason for hiding this comment

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

For me, the main question is "do we want to have 𝕜 in this file?"
All useful theorems only work for 𝕜 = ℝ. See https://leanprover.zulipchat.com/#narrow/stream/144837-PR-reviews/topic/.2311794.20Strictly.20convex.20spaces
The proof of the implication "unit ball → any ball" only works for 𝕜 = ℝ. Not sure about a counterexample.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Proving strict convexity of a ball at the origin is significantly easier than proving strict convexity of the unit ball, because you can use the norm in place of the distance to 0. Proving strict convexity of the unit ball does not seem significantly easier than proving strict convexity of any ball at the origin.

I believe 𝕜 = ℚ also works, because the main restriction to "unit ball → any ball" is that your points might reach only a discrete set of norms. Hence I believe some polynomial ring over some space will make "unit ball → any ball" fail.

@sgouezel
Copy link
Collaborator

bors r+
Thanks!

@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 Mar 26, 2022
bors bot pushed a commit that referenced this pull request Mar 26, 2022
…1794)

Define `strictly_convex_space`, a `Prop`-valued mixin to state that a normed space is strictly convex.



Co-authored-by: Yury G. Kudryashov <urkud@urkud.name>
@bors
Copy link

bors bot commented Mar 26, 2022

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title feat(analysis/convex/strict_convex_space): Strictly convex spaces [Merged by Bors] - feat(analysis/convex/strict_convex_space): Strictly convex spaces Mar 26, 2022
@bors bors bot closed this Mar 26, 2022
@bors bors bot deleted the strict_convex_space branch March 26, 2022 21:15
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+`.)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants