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(order/upper_lower): Reverse the order on upper_set #14982

Closed
wants to merge 3 commits into from

Conversation

YaelDillies
Copy link
Collaborator

Having upper_set being ordered by reverse inclusion makes it order-isomorphic to lower_set (and antichains once we have them as a type) and it matches the order on filter.


Open in Gitpod

@YaelDillies YaelDillies added the awaiting-review The author would like community review of the PR label Jun 26, 2022
src/order/upper_lower.lean Outdated Show resolved Hide resolved
src/order/upper_lower.lean Outdated Show resolved Hide resolved
Co-authored-by: Violeta Hernández <vi.hdz.p@gmail.com>
@b-mehta
Copy link
Collaborator

b-mehta commented Jun 26, 2022

I think it's worth adding a docstring somewhere explaining the rationale for the change: your justification is sound to me, but I can imagine others reading the library later and not realising the purpose

src/order/upper_lower.lean Outdated Show resolved Hide resolved
by simp_rw lower_set.compl_infi

end lower_set

/-- Upper sets are order-isomorphic to lower sets under complementation. -/
@[simps] def upper_set_iso_lower_set : upper_set α ≃o lower_set α :=
Copy link
Collaborator

Choose a reason for hiding this comment

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

If you put this just after compl_le_compl, does it give one-line proofs of compl_top and all of those lemmas?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It would, but the proofs still would end up being longer by virtue of upper_set_iso_lower_set being a long name. Further, I can't really see how to incorporate this in the flow of the file, given that it would need to be out of the namespaces but come between upper_set.compl/lower_set.compl and their API.

@b-mehta
Copy link
Collaborator

b-mehta commented Jun 26, 2022

bors d+

@bors
Copy link

bors bot commented Jun 26, 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.

@github-actions github-actions bot 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 Jun 26, 2022
@YaelDillies
Copy link
Collaborator Author

Thanks Bhavik!

bors merge

bors bot pushed a commit that referenced this pull request Jun 27, 2022
Having `upper_set` being ordered by reverse inclusion makes it order-isomorphic to `lower_set` (and antichains once we have them as a type) and it matches the order on `filter`.
@bors
Copy link

bors bot commented Jun 27, 2022

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title refactor(order/upper_lower): Reverse the order on upper_set [Merged by Bors] - refactor(order/upper_lower): Reverse the order on upper_set Jun 27, 2022
@bors bors bot closed this Jun 27, 2022
@bors bors bot deleted the reverse_order_upper_set branch June 27, 2022 00:49
@@ -370,40 +386,39 @@ def Ioi (a : α) : upper_set α := ⟨Ioi a, is_upper_set_Ioi a⟩
@[simp] lemma mem_Ici_iff : b ∈ Ici a ↔ a ≤ b := iff.rfl
@[simp] lemma mem_Ioi_iff : b ∈ Ioi a ↔ a < b := iff.rfl

lemma Ioi_le_Ici (a : α) : Ioi a ≤ Ici a := Ioi_subset_Ici_self
lemma Icoi_le_Ioi (a : α) : Ici a ≤ Ioi a := Ioi_subset_Ici_self
Copy link
Member

Choose a reason for hiding this comment

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

Typo?

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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants