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] - fix(order/complete_lattice): fix diamond in sup vs max and min vs inf #11309
[Merged by Bors] - fix(order/complete_lattice): fix diamond in sup vs max and min vs inf #11309
Conversation
This PR/issue depends on: |
src/data/real/ennreal.lean
Outdated
{ max_def := rfl, | ||
min_def := rfl, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have no idea why rfl
works here but the reflexivity
auto_param
does not. Either way, I'm inclined not to care. I can leave a TODO comment if someone else cares.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These lines create a new diamond: now max
on ennreal
is not defeq sup
, so we'll have two sup
s. Please derive linear_order
and use it here. Or just use .. (infer_instance : linear_order ennreal)
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, I must not have been paying attention. derive
is the right choice here.
(or.inl ∘ coe_le_coe.2))), | ||
(or.inl ∘ coe_le_coe.2))) } | ||
|
||
noncomputable instance : linear_order enat := |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that we have lattice.to_linear_order
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think a full lattice is available yet?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks 🎉
bors merge
…#11309) `complete_linear_order` has separate `max` and `sup` fields. There is no need to have both, so this removes one of them by telling the structure compiler to merge the two fields. The same is true of `inf` and `min`. As well as diamonds being possible in the abstract case, a handful of concrete example of this diamond existed. `linear_order` instances with default-populated fields were usually to blame.
Build failed (retrying...): |
…#11309) `complete_linear_order` has separate `max` and `sup` fields. There is no need to have both, so this removes one of them by telling the structure compiler to merge the two fields. The same is true of `inf` and `min`. As well as diamonds being possible in the abstract case, a handful of concrete example of this diamond existed. `linear_order` instances with default-populated fields were usually to blame.
Pull request successfully merged into master. Build succeeded: |
complete_linear_order
has separatemax
andsup
fields. There is no need to have both, so this removes one of them by telling the structure compiler to merge the two fields. The same is true ofinf
andmin
.As well as diamonds being possible in the abstract case, a handful of concrete example of this diamond existed.
linear_order
instances with default-populated fields were usually to blame.