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

feat (order/modular_lattice): modular_lattice as extension of lattice #7539

Closed
wants to merge 1 commit into from

Conversation

Seeram
Copy link

@Seeram Seeram commented May 6, 2021


Open in Gitpod

@Seeram Seeram added the awaiting-review The author would like community review of the PR label May 6, 2021
@@ -36,6 +36,9 @@ variable {α : Type*}
class is_modular_lattice α [lattice α] : Prop :=
(sup_inf_le_assoc_of_le : ∀ {x : α} (y : α) {z : α}, x ≤ z → (x ⊔ y) ⊓ z ≤ x ⊔ (y ⊓ z))

class modular_lattice α extends lattice α :=
(modular_law: ∀ (x u v : α ), (x ≤ u) → u ⊓ (v ⊔ x) = (u ⊓ v) ⊔ x )
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this class is a good idea - it means you need a new class to talk about modular bounded lattices, modular complete lattices, ...

What was the reason you added it?

namespace modular_lattice

theorem diamond_isomorphism
[modular_lattice α] {u v w x y : α} :
Copy link
Member

Choose a reason for hiding this comment

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

Instead of a new class, the existing one should work fine

Suggested change
[modular_lattice α] {u v w x y : α} :
[lattice α] [is_modular_lattice α] {u v w x y : α} :

{ rw modular_lattice.modular_law,
exact sup_eq_right.mpr h3,
exact h1 },
{ rw ← modular_lattice.modular_law,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
{ rw ← modular_lattice.modular_law,
{ rw ← inf_sup_assoc_of_le,

Or similar, with the changes I suggest above

@eric-wieser eric-wieser added awaiting-author A reviewer has asked the author a question or requested changes and removed awaiting-review The author would like community review of the PR labels May 6, 2021

theorem diamond_isomorphism
[modular_lattice α] {u v w x y : α} :
(x ≤ u) → (x ≥ v) → (x ≥ u ⊓ v) → (x ≤ u ⊔ v) → u ⊓ (v ⊔ x) = x ∧ (x ⊓ u) ⊔ v = x :=
Copy link
Member

Choose a reason for hiding this comment

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

Mathlib almost never uses :

Suggested change
(x ≤ u) → (x ≥ v) → (x ≥ u ⊓ v) → (x ≤ u ⊔ v) → u ⊓ (v ⊔ x) = x ∧ (x ⊓ u) ⊔ v = x :=
(x ≤ u) → (v ≤ x) → (u ⊓ v ≤ x) → (x ≤ u ⊔ v) → u ⊓ (v ⊔ x) = x ∧ (x ⊓ u) ⊔ v = x :=

Comment on lines +92 to +94
{ rw modular_lattice.modular_law,
exact sup_eq_right.mpr h3,
exact h1 },
Copy link
Member

Choose a reason for hiding this comment

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

This half of the proof only uses h1 and h3, while the other branch only uses h2 and h4. This suggests to me that this should be two different lemmas not a single lemma.

@bryangingechen
Copy link
Collaborator

@YaelDillies Do you mind posting a comment summarizing the reason(s) for closing the PR?

@YaelDillies
Copy link
Collaborator

There are too many types of modular lattices for us to insert them in the order hierarchy. Instead, I envision Prop-valued mixins as the way to go. See #11602

@YaelDillies YaelDillies deleted the modular_lattice_isomorphism branch October 31, 2022 07:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-author A reviewer has asked the author a question or requested changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants