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/order/monoid): Co/contravariant classes for with_bot/with_top #13369

Closed
wants to merge 7 commits into from

Conversation

YaelDillies
Copy link
Collaborator

@YaelDillies YaelDillies commented Apr 11, 2022

Add the covariant_class (with_bot α) (with_bot α) (+) (≤) and contravariant_class (with_bot α) (with_bot α) (+) (<) instances, as well as the lemmas that covariant_class (with_bot α) (with_bot α) (+) (<) and contravariant_class (with_bot α) (with_bot α) (+) (≤) almost hold.

On the way, match the APIs for with_bot/with_top by adding missing lemmas.

Co-authored-by: Wrenna Robson wren.robson@gmail.com


Sorry, the diff is a bit mangled because I had to move with_top/with_bot down the file to use order_dual. Note that I am not adding with_top.add_lt_add on purpose. We are currently unsure what the correct typeclass assumptions and whether we are not requiring a new one.

Open in Gitpod

@YaelDillies YaelDillies added the awaiting-review The author would like community review of the PR label Apr 11, 2022
@adomani
Copy link
Collaborator

adomani commented Apr 15, 2022

I think that this looks very nice!

Why did you do the with_top/bot for has_add and not for @[to_additive] has_mul? Is it already available somewhere else? I'm thinking of nnreal/ennreal where it might be useful to get covariant assumptions for addition and multiplication at the same time.

@YaelDillies
Copy link
Collaborator Author

with_top and with_bot do not have multiplication by default. I believe this is because that would disagree with the one on ennreal as 0 * ∞ = ∞ * 0 = 0 (instead of 0 * ∞ = ∞ * 0 = ∞).

@adomani
Copy link
Collaborator

adomani commented Apr 15, 2022

Ah, I see! Thanks for the explanation!

@adomani
Copy link
Collaborator

adomani commented Apr 15, 2022

There starts being quite a bit of code-repetition floating around co(ntra)variant (some operation) and co(ntra)variant (swap (some operation)) (not just here, everywhere, I would say). I wonder if we could get mul_opposites to take care of such things. E.g, if a type R satisfies covariant_class R R (*) (<) then R^op satisfies covariant_class R R (swap (*)) (<).

@adomani
Copy link
Collaborator

adomani commented Apr 15, 2022

Anyway, in case this is relevant, it would be for a separate PR!

LGTM!

Copy link
Collaborator

@b-mehta b-mehta left a comment

Choose a reason for hiding this comment

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

bors merge

@leanprover-community-bot-assistant leanprover-community-bot-assistant added ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.) and removed awaiting-review The author would like community review of the PR labels Apr 16, 2022
bors bot pushed a commit that referenced this pull request Apr 16, 2022
…with_top` (#13369)

Add the `covariant_class (with_bot α) (with_bot α) (+) (≤)` and `contravariant_class (with_bot α) (with_bot α) (+) (<)` instances, as well as the lemmas that `covariant_class (with_bot α) (with_bot α) (+) (<)` and `contravariant_class (with_bot α) (with_bot α) (+) (≤)` almost hold.

On the way, match the APIs for `with_bot`/`with_top` by adding missing lemmas.

Co-authored-by: Wrenna Robson <wren.robson@gmail.com>
@bors
Copy link

bors bot commented Apr 16, 2022

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title feat(algebra/order/monoid): Co/contravariant classes for with_bot/with_top [Merged by Bors] - feat(algebra/order/monoid): Co/contravariant classes for with_bot/with_top Apr 16, 2022
@bors bors bot closed this Apr 16, 2022
@bors bors bot deleted the with_bot_top_covariant branch April 16, 2022 19:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants