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] - refactor: Make CompleteLinearOrder extend BiheytingAlgebra #12731

Closed
wants to merge 6 commits into from

Conversation

YaelDillies
Copy link
Collaborator

@YaelDillies YaelDillies commented May 7, 2024

In fact, linear orders themselves are biheyting algebras, but registering this as an instance causes a bit of a chicken and the egg problem which I'm not willing to try solving. CompleteLinearOrder really does need to extend BiheytingAlgebra if we are going to make Frame extend HeytingAlgebra.

This is a step towards #10560


Open in Gitpod

In fact, linear orders themselves are biheyting algebras, but registering this as an instance causes a bit of a chicken and the egg problem which I'm not willing to try solving. `CompleteLinearOrder` really does need to extend `BiheytingAlgebra` if we are going to make `Frame` extend `HeytingAlgebra`.
Comment on lines 1041 to 1043
decidableLE := Classical.decRel _
decidableEq := Classical.decRel _
decidableLT := Classical.decRel _
Copy link
Member

Choose a reason for hiding this comment

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

I worry this is going to create a diamond with the instLinearOrder instance above. Can't you use __ := instLinearOrder here?

Copy link
Collaborator Author

@YaelDillies YaelDillies May 7, 2024

Choose a reason for hiding this comment

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

Both are definitely Classical.decRel _ because they prove decidability of inclusions of sets.

Spread notation seems to be broken/uncover a diamond. This works:

noncomputable instance UpperSet.instCompleteLinearOrder : CompleteLinearOrder (UpperSet α) where
  __ := completelyDistribLattice
  __ := instLinearOrder
  himp := LinearOrder.toBiheytingAlgebra.himp
  le_himp_iff := LinearOrder.toBiheytingAlgebra.le_himp_iff
  compl := LinearOrder.toBiheytingAlgebra.compl
  himp_bot := LinearOrder.toBiheytingAlgebra.himp_bot
  sdiff := LinearOrder.toBiheytingAlgebra.sdiff
  hnot := LinearOrder.toBiheytingAlgebra.hnot
  sdiff_le_iff := LinearOrder.toBiheytingAlgebra.sdiff_le_iff
  top_sdiff := LinearOrder.toBiheytingAlgebra.top_sdiff

This works too:

noncomputable instance UpperSet.instCompleteLinearOrder : CompleteLinearOrder (UpperSet α) :=
  { completelyDistribLattice, instLinearOrder, LinearOrder.toBiheytingAlgebra with }

This doesn't:

noncomputable instance UpperSet.instCompleteLinearOrder : CompleteLinearOrder (UpperSet α) where
  __ := completelyDistribLattice
  __ := instLinearOrder
  __ := LinearOrder.toBiheytingAlgebra

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it possible to have "tests" to confirm the definitional equality (with_reducible_and_instances rfl?) of diamonds?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

How would that help here? I think it's a metavariable issue with the spread notation, not an actual non-defeq diamond.

Copy link
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 🎉

bors merge

mathlib-bors bot pushed a commit that referenced this pull request Jun 24, 2024
In fact, linear orders themselves are biheyting algebras, but registering this as an instance causes a bit of a chicken and the egg problem which I'm not willing to try solving. `CompleteLinearOrder` really does need to extend `BiheytingAlgebra` if we are going to make `Frame` extend `HeytingAlgebra`.

This is a step towards #10560
@mathlib-bors
Copy link
Contributor

mathlib-bors bot commented Jun 24, 2024

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title refactor: Make CompleteLinearOrder extend BiheytingAlgebra [Merged by Bors] - refactor: Make CompleteLinearOrder extend BiheytingAlgebra Jun 24, 2024
@mathlib-bors mathlib-bors bot closed this Jun 24, 2024
@mathlib-bors mathlib-bors bot deleted the complete_lattice_extends_heyting_algebra branch June 24, 2024 17:57
kbuzzard pushed a commit that referenced this pull request Jun 26, 2024
In fact, linear orders themselves are biheyting algebras, but registering this as an instance causes a bit of a chicken and the egg problem which I'm not willing to try solving. `CompleteLinearOrder` really does need to extend `BiheytingAlgebra` if we are going to make `Frame` extend `HeytingAlgebra`.

This is a step towards #10560
dagurtomas pushed a commit that referenced this pull request Jul 2, 2024
In fact, linear orders themselves are biheyting algebras, but registering this as an instance causes a bit of a chicken and the egg problem which I'm not willing to try solving. `CompleteLinearOrder` really does need to extend `BiheytingAlgebra` if we are going to make `Frame` extend `HeytingAlgebra`.

This is a step towards #10560
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
t-order Order theory
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants