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] - feat(Finset): add gcongr attributes #9520

Closed
wants to merge 11 commits into from

Conversation

urkud
Copy link
Member

@urkud urkud commented Jan 7, 2024


@hrmacbeth In some cases I had to add equivalent lemmas to make @[gcongr] happy.
Should it accept iff lemmas and/or Monotone/Antitone/StrictMono/StrictAnti lemmas or it's hard to implement without hitting performance?

Open in Gitpod

Mathlib/Data/Finset/Basic.lean Outdated Show resolved Hide resolved
@urkud urkud added the t-logic Logic (model theory, set theory, etc) label Jan 25, 2024
theorem filter_subset_filter {s t : Finset α} (h : s ⊆ t) : s.filter p ⊆ t.filter p := fun _a ha =>
mem_filter.2 ⟨h (mem_filter.1 ha).1, (mem_filter.1 ha).2⟩
#align finset.filter_subset_filter Finset.filter_subset_filter

theorem monotone_filter_left : Monotone (filter p) := fun _ _ => filter_subset_filter p
#align finset.monotone_filter_left Finset.monotone_filter_left

-- TODO: `@[gcongr]` doesn't accept this lemma because of the `DecidablePred` arguments
theorem monotone_filter_right (s : Finset α) ⦃p q : α → Prop⦄ [DecidablePred p] [DecidablePred q]
Copy link
Member

Choose a reason for hiding this comment

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

This is interesting, I forgot we had this order on Prop. I wonder if there's a reimplementation of grw which exploits this more seriously.

If you want to make it gcongr, should we make a copy of this lemmas which uses classical reasoning and omits the [DecidablePred] hypotheses? (Would that work?)

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess, this would work but only if we're dealing with classical DecidablePreds in Finset.filter.

simp only -- Need to unfold `xs` and do alpha reduction, otherwise `gcongr` fails
gcongr
refine ⟨xs, h_mono, tendsto_atTop_mono (fun n ↦ Finset.le_sup' _ ?_) h⟩
simp
Copy link
Member

Choose a reason for hiding this comment

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

Nice!!!

@hrmacbeth
Copy link
Member

@hrmacbeth In some cases I had to add equivalent lemmas to make @[gcongr] happy.
Should it accept iff lemmas and/or Monotone/Antitone/StrictMono/StrictAnti lemmas or it's hard to implement without hitting performance?

I think these would all be reasonable. (mono used to handle Monotone.) Just lack of time, as yet, to implement them.

@jcommelin
Copy link
Member

LGTM

maintainer merge

Copy link

🚀 Pull request has been placed on the maintainer queue by jcommelin.

@urkud urkud requested a review from ericrbg February 23, 2024 05:47
@@ -1135,6 +1143,12 @@ theorem inf'_mono {s₁ s₂ : Finset β} (h : s₁ ⊆ s₂) (h₁ : s₁.Nonem
s₂.inf' (h₁.mono h) f ≤ s₁.inf' h₁ f :=
Finset.le_inf' h₁ _ (fun _ hb => inf'_le _ (h hb))

/-- A version of `Finset.inf'_mono` acceptable for `@[gcongr]`. -/
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't remember what I wrote here precisely, probably it was asking for slightly more explanation which changes were needed: is it just that h₂ is added or also that h₁ and h₂ are marked as implicit now?

@Vierkantor
Copy link
Contributor

Not sure why I wanted to await-author here, I think with a comment we can be done.

bors d+

@mathlib-bors
Copy link
Contributor

mathlib-bors bot commented Feb 28, 2024

✌️ urkud can now approve this pull request. To approve and merge a pull request, simply reply with bors r+. More detailed instructions are available here.

@urkud
Copy link
Member Author

urkud commented Feb 29, 2024

bors merge

@github-actions github-actions bot added the ready-to-merge This PR has been sent to bors. label Feb 29, 2024
mathlib-bors bot pushed a commit that referenced this pull request Feb 29, 2024
Co-authored-by: Johan Commelin <johan@commelin.net>
@mathlib-bors
Copy link
Contributor

mathlib-bors bot commented Feb 29, 2024

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title feat(Finset): add gcongr attributes [Merged by Bors] - feat(Finset): add gcongr attributes Feb 29, 2024
@mathlib-bors mathlib-bors bot closed this Feb 29, 2024
@mathlib-bors mathlib-bors bot deleted the YK-finset-gcongr branch February 29, 2024 02:01
mathlib-bors bot pushed a commit that referenced this pull request Feb 29, 2024
I accidentally merged #9520
before pushing these changes requested by @Vierkantor
riccardobrasca pushed a commit that referenced this pull request Mar 1, 2024
Co-authored-by: Johan Commelin <johan@commelin.net>
riccardobrasca pushed a commit that referenced this pull request Mar 1, 2024
I accidentally merged #9520
before pushing these changes requested by @Vierkantor
kbuzzard pushed a commit that referenced this pull request Mar 12, 2024
Co-authored-by: Johan Commelin <johan@commelin.net>
kbuzzard pushed a commit that referenced this pull request Mar 12, 2024
I accidentally merged #9520
before pushing these changes requested by @Vierkantor
dagurtomas pushed a commit that referenced this pull request Mar 22, 2024
Co-authored-by: Johan Commelin <johan@commelin.net>
dagurtomas pushed a commit that referenced this pull request Mar 22, 2024
I accidentally merged #9520
before pushing these changes requested by @Vierkantor
utensil pushed a commit that referenced this pull request Mar 26, 2024
Co-authored-by: Johan Commelin <johan@commelin.net>
utensil pushed a commit that referenced this pull request Mar 26, 2024
I accidentally merged #9520
before pushing these changes requested by @Vierkantor
Louddy pushed a commit that referenced this pull request Apr 15, 2024
Co-authored-by: Johan Commelin <johan@commelin.net>
Louddy pushed a commit that referenced this pull request Apr 15, 2024
I accidentally merged #9520
before pushing these changes requested by @Vierkantor
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
delegated maintainer-merge ready-to-merge This PR has been sent to bors. t-logic Logic (model theory, set theory, etc)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants