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: use suffix for lemmas with positivity assumptions, create aliases #9249

Closed
wants to merge 10 commits into from

Conversation

FR-vdash-bot
Copy link
Collaborator

@FR-vdash-bot FR-vdash-bot commented Dec 23, 2023

This PR uses the suffix on some lemmas which assume positivity.

This allows the names to be clearly distinguished from versions that do not require positivity assumptions, and also do not become too long.

I've kept most of the previous names (short names and long names suffixed with _of_pos or _of_nonneg) as aliases, but not for wrong names and confusing names.

Due to name conflicts, some similar lemmas have dissimilar names before this PR:

theorem mul_lt_mul_of_nonneg_of_pos [PosMulMono α] [MulPosStrictMono α]
    (h₁ : a < b) (h₂ : c ≤ d) (a0 : 0 ≤ a) (d0 : 0 < d) : a * c < b * d
theorem mul_lt_mul_of_le_of_lt' [PosMulMono α] [MulPosStrictMono α]
    (h₁ : a < b) (h₂ : c ≤ d) (b0 : 0 ≤ b) (c0 : 0 < c) : a * c < b * d

If there is a better way to resolve name conflicts, please feel free to comment.


Ported from leanprover-community/mathlib3#16523

Open in Gitpod

@FR-vdash-bot FR-vdash-bot added WIP Work in progress awaiting-review awaiting-CI t-algebra Algebra (groups, rings, fields, etc) t-order Order theory labels Dec 23, 2023
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Dec 26, 2023
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot removed the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Dec 28, 2023
@FR-vdash-bot FR-vdash-bot removed the WIP Work in progress label Dec 28, 2023
#align mul_lt_mul_of_lt_of_lt' mul_lt_mul_of_lt_of_ltₚ'

alias mul_le_mul := mul_le_mul_of_le_of_leₚ' -- this name was in `Algebra.Order.Ring.Defs`
alias mul_lt_mul := mul_lt_mul_of_lt_of_leₚ' -- this name was in `Algebra.Order.Ring.Defs`
Copy link
Collaborator

Choose a reason for hiding this comment

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

These statements are not identical. mul_lt_mul holds in a StrictOrderedSemiring. Of course, the lemma you are aliasing holds in more generality, but it also makes Lean work harder to synthesize type classes. I am dubious whether we are actually okay with this unless I see a !bench showing the effect is negligible.

I've highlighted mul_lt_mul, but I suspect the same is true for many of the aliases you have in this PR.

@j-loreaux j-loreaux added the RFC Request for comment label Jan 18, 2024
@j-loreaux
Copy link
Collaborator

I've opened a Zulip thread because I think this is a somewhat significant change to our naming conventions and as such I've marked this as RFC.

@FR-vdash-bot
Copy link
Collaborator Author

!bench

@leanprover-bot
Copy link
Collaborator

Here are the benchmark results for commit 7559b09.
There were no significant changes against commit fb3fd7c.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) RFC Request for comment t-algebra Algebra (groups, rings, fields, etc) t-order Order theory
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants