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(algebra/*): Algebraic instances on lex/order_dual #16122

Closed
wants to merge 14 commits into from

Conversation

YaelDillies
Copy link
Collaborator

@YaelDillies YaelDillies commented Aug 18, 2022

Copy over algebraic instances for lex and order_dual.


Open in Gitpod

@YaelDillies YaelDillies added awaiting-review The author would like community review of the PR t-algebra Algebra (groups, rings, fields etc) t-order Order hierarchy labels Aug 18, 2022
@adomani
Copy link
Collaborator

adomani commented Aug 18, 2022

I can already hear Eric talking about of_lex 1...

Copy link
Collaborator

@adomani adomani left a comment

Choose a reason for hiding this comment

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

Besides the lemmas about to/of_lex 1 and the left/right_cancel_semigroups, this looks geat!

Thanks!

Copy link
Member

@eric-wieser eric-wieser left a comment

Choose a reason for hiding this comment

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

Is it worth adding division_ring/field and rat_cast too?

bors d+

@bors
Copy link

bors bot commented Aug 19, 2022

✌️ YaelDillies can now approve this pull request. To approve and merge a pull request, simply reply with bors r+. More detailed instructions are available here.

@leanprover-community-bot-assistant leanprover-community-bot-assistant added delegated The PR author may merge after reviewing final suggestions. and removed awaiting-review The author would like community review of the PR labels Aug 19, 2022
@YaelDillies YaelDillies changed the title feat(algebra/order/{monoid,group}): Algebraic instances on lex feat(algebra/*): Algebraic instances on lex/order_dual Aug 19, 2022
@YaelDillies
Copy link
Collaborator Author

I moved the instances closer to the definitions of the corresponding typeclasses because the pattern of putting instances that are defined in algebra.X.something in algebra.order.X breaks with smul (as algebra.order.smul imports algebra.order.monoid but has_smul and monoid are both defined in algebra.group.defs).


open order_dual

@[to_additive] instance : Π [has_one α], has_one αᵒᵈ := id
Copy link
Member

Choose a reason for hiding this comment

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

@urkud said that using id was a bad idea since it's one more thing to unfold

Suggested change
@[to_additive] instance : Π [has_one α], has_one αᵒᵈ := id
@[to_additive] instance [has_one α] : has_one αᵒᵈ := ‹has_one α›

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For future reference, here's a regex that works to turn one into the other: : Π \[(.*)\],(.*)id, [h : $1] :$2h


open order_dual

@[to_additive] instance [h : has_one α] : has_one αᵒᵈ := h
Copy link
Member

Choose a reason for hiding this comment

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

Note that

Suggested change
@[to_additive] instance [h : has_one α] : has_one αᵒᵈ := h
@[to_additive] instance [has_one α] : has_one αᵒᵈ := ‹has_one α›

avoids h : appearing in the docs; but perhaps this is too minor to care about.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, it's mostly harder to industrialise because of the line-wrapping 😦

@YaelDillies
Copy link
Collaborator Author

bors merge

@bors
Copy link

bors bot commented Aug 21, 2022

Merge conflict.

@YaelDillies
Copy link
Collaborator Author

Let's try again

bors merge

bors bot pushed a commit that referenced this pull request Aug 21, 2022
Copy over algebraic instances for `lex` and `order_dual`.
@bors
Copy link

bors bot commented Aug 21, 2022

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title feat(algebra/*): Algebraic instances on lex/order_dual [Merged by Bors] - feat(algebra/*): Algebraic instances on lex/order_dual Aug 21, 2022
@bors bors bot closed this Aug 21, 2022
@bors bors bot deleted the lex_algebraic_instances branch August 21, 2022 12:50
bors bot pushed a commit that referenced this pull request Oct 15, 2022
#16847 copied over the copyright from `algebra.group.basic` without actually attributing things properly.

All the material in `algebra.group.commutator` is from #12309 and the material is mostly from #16122 with some dating back from #8564 

Also fix `order_dual.has_pow` being additivized as `order_dual.has_nsmul` rather than `order_dual.has_smul`.
bors bot pushed a commit that referenced this pull request Oct 15, 2022
…e names (#16967)

#16847 copied over the copyright from `algebra.group.basic` without actually attributing things properly.

All the material in `algebra.group.commutator` is from #12309 and the material is mostly from #16122 with some dating back from #8564 

Also fix `order_dual.has_pow` being additivized as `order_dual.has_nsmul` rather than `order_dual.has_smul`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
delegated The PR author may merge after reviewing final suggestions. t-algebra Algebra (groups, rings, fields etc) t-order Order hierarchy
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants