Skip to content

[Merged by Bors] - feat: port Algebra.Order.Monoid.OrderDual#786

Closed
zeramorphic wants to merge 6 commits intomasterfrom
Algebra.Order.Monoid.OrderDual
Closed

[Merged by Bors] - feat: port Algebra.Order.Monoid.OrderDual#786
zeramorphic wants to merge 6 commits intomasterfrom
Algebra.Order.Monoid.OrderDual

Conversation

@zeramorphic
Copy link
Copy Markdown
Collaborator

mathlib3 SHA: 1fc36cc9c8264e6e81253f88be7fb2cb6c92d76a

Notes:

  1. On line 80, the elaborator tries to synthesise OrderedCancelCommMonoid αᵒᵈ and fails, instead of using OrderedCancelCommMonoid α as is intended - so I needed to write out the instance explicitly.

  2. A couple of @[to_additive]s needed to be added.

Signed-off-by: thirdsgames thirdsgames2018@gmail.com

Signed-off-by: thirdsgames <thirdsgames2018@gmail.com>
Signed-off-by: thirdsgames <thirdsgames2018@gmail.com>
@hrmacbeth
Copy link
Copy Markdown
Member

  1. On line 80, the elaborator tries to synthesise OrderedCancelCommMonoid αᵒᵈ and fails, instead of using OrderedCancelCommMonoid α as is intended - so I needed to write out the instance explicitly.

This is probably leanprover/lean4#1892, see Zulip yesterday for the same in Order.Disjoint.

@zeramorphic zeramorphic added mathlib-port This is a port of a theory file from mathlib. awaiting-review labels Nov 29, 2022
namespace OrderDual

@[to_additive]
instance contravariant_class_mul_le [LE α] [Mul α] [c : ContravariantClass α α (· * ·) (· ≤ ·)] :
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
instance contravariant_class_mul_le [LE α] [Mul α] [c : ContravariantClass α α (· * ·) (· ≤ ·)] :
instance contravariant_class_mul_le [LE α] [Mul α] [c : ContravariantClass α α (· * ·) (· ≤ ·)] :

This should be called contravariantClass_mul_le. Could you rename?

You'll also need to add #align statements for the additive versions. (Hopefully this can be automated, see leanprover-community/mathport#205.)

@kim-em
Copy link
Copy Markdown
Contributor

kim-em commented Nov 29, 2022

Thanks, @zeramorphic! It's very helpful if you can put comments in the source file starting with Porting note: that explain changes you needed to make. (It's easier to help fix them if they are noted in the file, and it also leaves us a better paper trail than just the PR descriptions.) In particular the line 80 issue should be a porting note.

@kim-em kim-em added awaiting-author A reviewer has asked the author a question or requested changes. and removed awaiting-review labels Nov 29, 2022
Signed-off-by: thirdsgames <thirdsgames2018@gmail.com>
@zeramorphic
Copy link
Copy Markdown
Collaborator Author

Thank you for your comments, I think I've fixed all the problems you outlined.

@zeramorphic zeramorphic added awaiting-review and removed awaiting-author A reviewer has asked the author a question or requested changes. labels Nov 29, 2022
@kim-em
Copy link
Copy Markdown
Contributor

kim-em commented Nov 29, 2022

🎉

bors merge

@github-actions github-actions bot added ready-to-merge This PR has been sent to bors. and removed awaiting-review labels Nov 29, 2022
bors bot pushed a commit that referenced this pull request Nov 29, 2022
mathlib3 SHA: 1fc36cc9c8264e6e81253f88be7fb2cb6c92d76a

Notes:

1. On line 80, the elaborator tries to synthesise `OrderedCancelCommMonoid αᵒᵈ` and fails, instead of using `OrderedCancelCommMonoid α` as is intended - so I needed to write out the instance explicitly.

2. A couple of `@[to_additive]`s needed to be added.

Signed-off-by: thirdsgames <thirdsgames2018@gmail.com>

Co-authored-by: Scott Morrison <scott.morrison@gmail.com>
@digama0 digama0 changed the title feat(Algebra.Order.Monoid.OrderDual): port file feat: port Algebra.Order.Monoid.OrderDual Nov 29, 2022
@bors
Copy link
Copy Markdown

bors bot commented Nov 29, 2022

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title feat: port Algebra.Order.Monoid.OrderDual [Merged by Bors] - feat: port Algebra.Order.Monoid.OrderDual Nov 29, 2022
@bors bors bot closed this Nov 29, 2022
@bors bors bot deleted the Algebra.Order.Monoid.OrderDual branch November 29, 2022 21:18
bors bot pushed a commit to leanprover-community/mathlib3 that referenced this pull request Dec 1, 2022
Regenerated from the [port status wiki page](https://github.com/leanprover-community/mathlib/wiki/mathlib4-port-status).
Relates to the following PRs:
* `algebra.field.defs`: leanprover-community/mathlib4#668
* `algebra.group.commute`: leanprover-community/mathlib4#750
* `algebra.group_with_zero.commute`: leanprover-community/mathlib4#762
* `algebra.group_with_zero.semiconj`: leanprover-community/mathlib4#757
* `algebra.group_with_zero.units.basic`: leanprover-community/mathlib4#735
* `algebra.hom.embedding`: leanprover-community/mathlib4#764
* `algebra.order.monoid.cancel.defs`: leanprover-community/mathlib4#774
* `algebra.order.monoid.canonical.defs`: leanprover-community/mathlib4#778
* `algebra.order.monoid.defs`: leanprover-community/mathlib4#771
* `algebra.order.monoid.min_max`: leanprover-community/mathlib4#763
* `algebra.order.monoid.order_dual`: leanprover-community/mathlib4#786
* `algebra.order.sub.defs`: leanprover-community/mathlib4#732
* `algebra.regular.basic`: leanprover-community/mathlib4#758
* `algebra.ring.commute`: leanprover-community/mathlib4#759
* `algebra.ring.regular`: leanprover-community/mathlib4#795
* `algebra.ring.semiconj`: leanprover-community/mathlib4#751
* `control.applicative`: leanprover-community/mathlib4#798
* `control.functor`: leanprover-community/mathlib4#612
* `control.monad.basic`: leanprover-community/mathlib4#752
* `data.countable.defs`: leanprover-community/mathlib4#736
* `data.int.units`: leanprover-community/mathlib4#807
* `data.nat.basic`: leanprover-community/mathlib4#729
* `data.nat.psub`: leanprover-community/mathlib4#806
* `data.nat.units`: leanprover-community/mathlib4#805
* `data.pi.algebra`: leanprover-community/mathlib4#564
* `data.prod.lex`: leanprover-community/mathlib4#783
* `logic.embedding.basic`: leanprover-community/mathlib4#700
* `logic.equiv.option`: leanprover-community/mathlib4#674
* `order.bounded_order`: leanprover-community/mathlib4#697
* `order.with_bot`: leanprover-community/mathlib4#776
* `order.disjoint`: leanprover-community/mathlib4#773
* `order.min_max`: leanprover-community/mathlib4#728
* `order.prop_instances`: leanprover-community/mathlib4#792
* `order.rel_iso.basic`: leanprover-community/mathlib4#772
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

mathlib-port This is a port of a theory file from mathlib. ready-to-merge This PR has been sent to bors.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants