Skip to content
This repository was archived by the owner on Jul 24, 2024. It is now read-only.

[Merged by Bors] - refactor(order/conditionally_complete_lattice): use order_bot#14568

Closed
urkud wants to merge 4 commits intomasterfrom
YK-wf-cclo
Closed

[Merged by Bors] - refactor(order/conditionally_complete_lattice): use order_bot#14568
urkud wants to merge 4 commits intomasterfrom
YK-wf-cclo

Conversation

@urkud
Copy link
Copy Markdown
Member

@urkud urkud commented Jun 6, 2022

Use order_bot instead of an explicit c = ⊥ argument in
well_founded.conditionally_complete_linear_order_with_bot. Also
reuse linear_order.to_lattice and add well_founded.min_le.


Open in Gitpod

urkud added 2 commits June 5, 2022 22:12
Use `order_bot` instead of an explicit `c = ⊥` argument in
`well_founded.conditionally_complete_linear_order_with_bot`. Also
reuse `linear_order.to_lattice`.
@urkud urkud added awaiting-review The author would like community review of the PR awaiting-CI The author would like to see what CI has to say before doing more work. labels Jun 6, 2022
{γ : Type*} [partial_order γ]

theorem min_le {x : β} {s : set β} (hx : x ∈ s) (hne : s.nonempty := ⟨x, hx⟩) :
h.min s hne ≤ x :=
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

hne doesn't need to be an argument, due to proof irrelevancy you can just put ⟨x, hx⟩ in the theorem statement and it will work just fine.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

That's why hne is an argument with a default value. AFAIR, it may help if we have h.min s hne with some other proof (though two proofs should be equal by definition, so I'm not sure).

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Yeah, that's why I suggest just inlining the proof. There's no circumstance in which you need to provide both hx and hne.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we've sometimes seen some weirdness where this behaves incorrectly due to simp or rw performing a syntactic match rather than unifying. This is definitely a pattern we've used elsewhere, but I don't think it ever got a library note to explain exactly why it matters.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I've inlined a bunch of proofs into a bunch of statements, and I've never had any of those problems. I'm interested in seeing an example where Lean attempts to do syntactic matching on a proof.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I've just met the following scenario. While proving disjoint (s : set α) (t : set α), I did rintro x ⟨hs, ht⟩ and got the goal x ∈ ⊥. Then I tried to write exact (hd : disjoint s' t') ⟨_, _⟩ and got x ∈ s' as the first goal. I can't quickly create an example for this lemma.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Well, if there's the possibility for breakage, then this auxiliary parameter can't possibly do harm. I might rewrite some of those other lemmas I was mentioning to use this pattern to. A library note would be much appreciated!

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'd be inclined to declare this out of scope, but maybe create a new issue about documenting this approach or cleaning it up

@github-actions github-actions bot removed the awaiting-CI The author would like to see what CI has to say before doing more work. label Jun 6, 2022
Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
@eric-wieser
Copy link
Copy Markdown
Member

bors merge

We could certainly still do with the library note in #14568 (comment), but that doesn't need to be in this PR.

@github-actions github-actions bot 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 Jun 13, 2022
bors bot pushed a commit that referenced this pull request Jun 13, 2022
Use `order_bot` instead of an explicit `c = ⊥` argument in
`well_founded.conditionally_complete_linear_order_with_bot`. Also
reuse `linear_order.to_lattice` and add `well_founded.min_le`.
@bors
Copy link
Copy Markdown

bors bot commented Jun 13, 2022

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title refactor(order/conditionally_complete_lattice): use order_bot [Merged by Bors] - refactor(order/conditionally_complete_lattice): use order_bot Jun 13, 2022
@bors bors bot closed this Jun 13, 2022
@bors bors bot deleted the YK-wf-cclo branch June 13, 2022 11:39
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

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.

3 participants