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] - chore(Algebra/Order/GroupWithZero/Unbundled): resolving name inconsistencies #9904

Closed
wants to merge 30 commits into from

Conversation

FR-vdash-bot
Copy link
Collaborator

@FR-vdash-bot FR-vdash-bot commented Jan 22, 2024

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

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
Statement New name Old name
a * c ≤ b * d mul_le_mul_of_nonneg mul_le_mul_of_le_of_le
mul_le_mul_of_nonneg' mul_le_mul (still there)
a * c < b * d mul_lt_mul_of_le_of_lt_of_pos_of_nonneg mul_lt_mul_of_pos_of_nonneg (still there)
mul_lt_mul_of_le_of_lt_of_nonneg_of_pos / mul_lt_mul_of_nonneg_of_pos' mul_lt_mul_of_le_of_le' / mul_lt_mul' (still there)
mul_lt_mul_of_lt_of_le_of_nonneg_of_pos mul_lt_mul_of_nonneg_of_pos (still there)
mul_lt_mul_of_lt_of_le_of_pos_of_nonneg / mul_lt_mul_of_pos_of_nonneg' mul_lt_mul_of_le_of_lt' / mul_lt_mul (still there)
mul_lt_mul_of_pos mul_lt_mul_of_pos_of_pos
mul_lt_mul_of_pos' mul_lt_mul_of_lt_of_lt'
(Left.)mul_lt_mul_of_nonneg mul_lt_mul'' (still there)
Right.mul_lt_mul_of_nonneg
a * b < 0 → 0 ≤ b → a < 0 (Left.)neg_of_mul_neg_left neg_of_mul_neg_left / Left.neg_of_mul_neg_right
Right.neg_of_mul_neg_left Right.neg_of_mul_neg_right
a * b < 0 → 0 ≤ a → b < 0 (Left.)neg_of_mul_neg_right neg_of_mul_neg_right / Left.neg_of_mul_neg_left
Right.neg_of_mul_neg_right Right.neg_of_mul_neg_left

Lines 304 - 321 and 476 - 495 were just moved together.

Some variable names are changed to be consistent with the rest of the file.

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


Split from #9249

Zulip

Open in Gitpod

@FR-vdash-bot FR-vdash-bot added awaiting-review t-algebra Algebra (groups, rings, fields, etc) t-order Order theory labels Jan 22, 2024
@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 Feb 12, 2024
@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 Mar 23, 2024
@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 Mar 28, 2024
@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 May 15, 2024
@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 May 22, 2024
@FR-vdash-bot FR-vdash-bot changed the title chore(Algebra/Order/Ring/Lemmas): resolving name inconsistencies chore(Algebra/Order/GroupWithZero/Unbundled): resolving name inconsistencies May 23, 2024
@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 May 23, 2024
@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 Jun 6, 2024
@kim-em kim-em requested a review from YaelDillies July 17, 2024 23:45
@YaelDillies
Copy link
Collaborator

I appreciate the effort. However your new names seem unnecessarily long (I know it gives more consistency, but the length really is unwieldy). Here are some guidelines:

  • op_rel_op_of_rel_of_rel should just be op_rel_op
  • _of_pred_of_pred can simply be _of_pred

This makes your table look like this

Statement New name Old name
a * c ≤ b * d mul_le_mul_of_nonneg mul_le_mul_of_le_of_le
mul_le_mul_of_nonneg' mul_le_mul (still there)
a * c < b * d mul_lt_mul_of_le_of_lt_of_pos_of_nonneg mul_lt_mul_of_pos_of_nonneg
mul_lt_mul_of_le_of_lt_of_nonneg_of_pos mul_lt_mul_of_le_of_le' / mul_lt_mul' (still there)
mul_lt_mul_of_lt_of_le_of_nonneg_of_pos mul_lt_mul_of_nonneg_of_pos
mul_lt_mul_of_lt_of_le_of_pos_of_nonneg mul_lt_mul_of_le_of_lt' / mul_lt_mul (still there)
mul_lt_mul_of_pos mul_lt_mul_of_pos_of_pos
mul_lt_mul_of_pos' mul_lt_mul_of_lt_of_lt'
(Left.)mul_lt_mul_of_nonneg mul_lt_mul'' (still there)
Right.mul_lt_mul_of_nonneg
a * b < 0 → 0 ≤ b → a < 0 (Left.)neg_of_mul_neg_left neg_of_mul_neg_left / Left.neg_of_mul_neg_right
Right.neg_of_mul_neg_left Right.neg_of_mul_neg_right
a * b < 0 → 0 ≤ a → b < 0 (Left.)neg_of_mul_neg_right neg_of_mul_neg_right / Left.neg_of_mul_neg_left
Right.neg_of_mul_neg_right Right.neg_of_mul_neg_left

There are still a few lines that are not great (eg mul_lt_mul_of_lt_of_le_of_nonneg_of_pos/mul_lt_mul_of_lt_of_le_of_nonneg_of_pos), but it's already much better.

@YaelDillies YaelDillies added the awaiting-author A reviewer has asked the author a question or requested changes label Jul 18, 2024
@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 Jul 19, 2024
@FR-vdash-bot FR-vdash-bot added awaiting-CI and removed awaiting-author A reviewer has asked the author a question or requested changes labels Jul 24, 2024
@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 Jul 24, 2024
@YaelDillies
Copy link
Collaborator

Could you keep mul_lt_mul_of_pos_of_nonneg/mul_lt_mul_of_nonneg_of_pos as aliases of the new names? I think they are still somewhat not ambiguous

@YaelDillies
Copy link
Collaborator

Thanks. This is now definitely an improvement.

maintainer merge

Copy link

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

@riccardobrasca
Copy link
Member

Thanks!

bors merge

@github-actions github-actions bot added the ready-to-merge This PR has been sent to bors. label Jul 25, 2024
mathlib-bors bot pushed a commit that referenced this pull request Jul 25, 2024
…tencies (#9904)

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

Due to name conflicts, some similar lemmas have dissimilar names before this PR:
``` lean
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
```

| Statement           | New name                | Old name                |
|

Co-authored-by: negiizhao <[email protected]>
Co-authored-by: FR <[email protected]>
@mathlib-bors
Copy link
Contributor

mathlib-bors bot commented Jul 25, 2024

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title chore(Algebra/Order/GroupWithZero/Unbundled): resolving name inconsistencies [Merged by Bors] - chore(Algebra/Order/GroupWithZero/Unbundled): resolving name inconsistencies Jul 25, 2024
@mathlib-bors mathlib-bors bot closed this Jul 25, 2024
@mathlib-bors mathlib-bors bot deleted the FR_order_refactor16' branch July 25, 2024 08:41
@adomani adomani mentioned this pull request Aug 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintainer-merge ready-to-merge This PR has been sent to bors. t-algebra Algebra (groups, rings, fields, etc) t-order Order theory
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants