Skip to content

[Merged by Bors] - refactor: Multiplicativise abs#9553

Closed
YaelDillies wants to merge 12 commits intomasterfrom
multiplicativise_abs
Closed

[Merged by Bors] - refactor: Multiplicativise abs#9553
YaelDillies wants to merge 12 commits intomasterfrom
multiplicativise_abs

Conversation

@YaelDillies
Copy link
Copy Markdown
Contributor

@YaelDillies YaelDillies commented Jan 8, 2024

The current design for abs is flawed:

  • The Abs notation typeclass has exactly two instances: one for [Neg α] [Sup α], one for [Inv α] [Sup α]. This means that:
    • We can't write a meaningful hover for Abs.abs
    • Fields have two Abs instances!
  • We have the multiplicative definition but:
    • All the lemmas in Algebra.Order.Group.Abs are about the additive version.
    • The only lemmas about the multiplicative version are in Algebra.Order.Group.PosPart, and they get additivised to duplicates of the lemmas in Algebra.Order.Group.Abs!

This PR changes the notation typeclass with two new definitions (related through to_additive): mabs and abs. abs inherits the |a| notation and mabs gets |a|ₘ instead.

The first half of Algebra.Order.Group.Abs gets multiplicativised. A later PR will multiplicativise the second half, and another one #9740 will deduplicate the lemmas in Algebra.Order.Group.PosPart.

Part of #9411. Closes #6376.


Open in Gitpod

The current design for `abs` is flawed:
* The `Abs` notation typeclass has exactly two instances: one for `[Neg α] [Sup α]`, one for `[Inv α] [Sup α]`. This means that:
  * We can't write a meaningful hover for `Abs.abs`
  * Fields have two `Abs` instances!
* We have the multiplicative definition but:
  * All the lemmas in `Algebra.Order.Group.Abs` are about the additive version.
  * The only lemmas about the multiplicative version are in `Algebra.Order.Group.PosPart`, and they get additivised to duplicates of the lemmas in `Algebra.Order.Group.Abs`!

This PR changes the notation typeclass with two new definitions (related through `to_additive`): `mabs` and `abs`. `abs` inherits the `|a|` notation and `mabs` gets `|a|ₘ` instead.

The first half of `Algebra.Order.Group.Abs` gets multiplicativised. A later PR will multiplicativise the second half,  and another one will deduplicate the lemmas in `Algebra.Order.Group.PosPart`.

Part of #9411
@YaelDillies YaelDillies added awaiting-review t-algebra Algebra (groups, rings, fields, etc) t-order Order theory labels Jan 8, 2024
@ghost ghost added the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Jan 9, 2024
@ghost ghost added merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) and removed merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) labels Jan 9, 2024
@ghost ghost removed the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Jan 9, 2024
@ghost ghost added the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Jan 10, 2024
@ghost ghost removed the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Jan 10, 2024
Copy link
Copy Markdown
Member

@jcommelin jcommelin left a comment

Choose a reason for hiding this comment

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

Thanks a lot for cleaning this up!!

bors merge

@ghost ghost added ready-to-merge This PR has been sent to bors. and removed awaiting-review labels Jan 13, 2024
mathlib-bors bot pushed a commit that referenced this pull request Jan 13, 2024
The current design for `abs` is flawed:
* The `Abs` notation typeclass has exactly two instances: one for `[Neg α] [Sup α]`, one for `[Inv α] [Sup α]`. This means that:
  * We can't write a meaningful hover for `Abs.abs`
  * Fields have two `Abs` instances!
* We have the multiplicative definition but:
  * All the lemmas in `Algebra.Order.Group.Abs` are about the additive version.
  * The only lemmas about the multiplicative version are in `Algebra.Order.Group.PosPart`, and they get additivised to duplicates of the lemmas in `Algebra.Order.Group.Abs`!

This PR changes the notation typeclass with two new definitions (related through `to_additive`): `mabs` and `abs`. `abs` inherits the `|a|` notation and `mabs` gets `|a|ₘ` instead.

The first half of `Algebra.Order.Group.Abs` gets multiplicativised. A later PR will multiplicativise the second half,  and another one will deduplicate the lemmas in `Algebra.Order.Group.PosPart`.

Part of #9411.




Co-authored-by: Jeremy Tan Jie Rui <reddeloostw@gmail.com>
@mathlib-bors
Copy link
Copy Markdown
Contributor

mathlib-bors bot commented Jan 13, 2024

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title refactor: Multiplicativise abs [Merged by Bors] - refactor: Multiplicativise abs Jan 13, 2024
@mathlib-bors mathlib-bors bot closed this Jan 13, 2024
@mathlib-bors mathlib-bors bot deleted the multiplicativise_abs branch January 13, 2024 15:06
YaelDillies added a commit that referenced this pull request Jan 14, 2024
This changes the typeclass notation approach with plain functions.

Followup to #9553
linesthatinterlace pushed a commit that referenced this pull request Jan 16, 2024
The current design for `abs` is flawed:
* The `Abs` notation typeclass has exactly two instances: one for `[Neg α] [Sup α]`, one for `[Inv α] [Sup α]`. This means that:
  * We can't write a meaningful hover for `Abs.abs`
  * Fields have two `Abs` instances!
* We have the multiplicative definition but:
  * All the lemmas in `Algebra.Order.Group.Abs` are about the additive version.
  * The only lemmas about the multiplicative version are in `Algebra.Order.Group.PosPart`, and they get additivised to duplicates of the lemmas in `Algebra.Order.Group.Abs`!

This PR changes the notation typeclass with two new definitions (related through `to_additive`): `mabs` and `abs`. `abs` inherits the `|a|` notation and `mabs` gets `|a|ₘ` instead.

The first half of `Algebra.Order.Group.Abs` gets multiplicativised. A later PR will multiplicativise the second half,  and another one will deduplicate the lemmas in `Algebra.Order.Group.PosPart`.

Part of #9411.




Co-authored-by: Jeremy Tan Jie Rui <reddeloostw@gmail.com>
mathlib-bors bot pushed a commit that referenced this pull request Jan 16, 2024
This changes the typeclass notation approach with plain functions.

Followup to #9553. Part of #9411
mathlib-bors bot pushed a commit that referenced this pull request Jan 30, 2024
`Algebra.GroupPower.Lemmas` used to be a big bag of lemmas that made it there on the criterion that they needed "more imports". This was completely untrue, as all lemmas could be moved to earlier files in PRs:
- #9440
- #9442
- #9443
- #9455
- #9456
- #9457
- #9459
- #9461
- #9463
- #9466
- #9501
- #9502
- #9503
- #9505
- #9551
- #9553
- #9720
- #9739
- #9740
- #9805
- #9806
- and this one

There are several reasons for this:
* Necessary lemmas have been moved to earlier files since lemmas were dumped in `Algebra.GroupPower.Lemmas`
* In the Lean 3 → Lean 4 transition, Std acquired basic `Int` and `Nat` lemmas which let us shortcircuit the part of the algebraic order hierarchy on which the corresponding general lemmas rest
* Some proofs were overpowered
* Some earlier files were tangled and I have untangled them

This PR finishes the job by moving the last few lemmas out of `Algebra.GroupPower.Lemmas`, which is therefore deleted.
mathlib-bors bot pushed a commit that referenced this pull request Jan 30, 2024
`Algebra.GroupPower.Lemmas` used to be a big bag of lemmas that made it there on the criterion that they needed "more imports". This was completely untrue, as all lemmas could be moved to earlier files in PRs:
- #9440
- #9442
- #9443
- #9455
- #9456
- #9457
- #9459
- #9461
- #9463
- #9466
- #9501
- #9502
- #9503
- #9505
- #9551
- #9553
- #9720
- #9739
- #9740
- #9805
- #9806
- and this one

There are several reasons for this:
* Necessary lemmas have been moved to earlier files since lemmas were dumped in `Algebra.GroupPower.Lemmas`
* In the Lean 3 → Lean 4 transition, Std acquired basic `Int` and `Nat` lemmas which let us shortcircuit the part of the algebraic order hierarchy on which the corresponding general lemmas rest
* Some proofs were overpowered
* Some earlier files were tangled and I have untangled them

This PR finishes the job by moving the last few lemmas out of `Algebra.GroupPower.Lemmas`, which is therefore deleted.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

Refactor of Algebra/Order/LatticeGroup with Algebra/Order/Group/Abs

3 participants