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(SetTheory): Upper bound for games #10566

Closed
wants to merge 2 commits into from

Conversation

YaelDillies
Copy link
Collaborator

Partially forward-port leanprover-community/mathlib#15260


Open in Gitpod

@YaelDillies YaelDillies added awaiting-review The author would like community review of the PR mathlib3-pair This PR is a forward-port of a mathlib3 PR or part of one, either under review or recently merged t-logic Logic (model theory, set theory, etc) labels Feb 14, 2024
@YaelDillies YaelDillies changed the title feat(SetTheory/Game): define PGame.upperBound feat(SetTheory): Upper bound for games Feb 14, 2024
Copy link
Collaborator

@timotree3 timotree3 left a comment

Choose a reason for hiding this comment

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

Thanks for the forward port! I'm trying to pitch in with a review as someone with an interest in CGT. This is indeed a faithful forward port of the pgame.lean changes in leanprover-community/mathlib#15260, so while I do have some requested changes, I don't mind if we merge this and save my feedback for a future PR.

Mathlib/SetTheory/Game/PGame.lean Outdated Show resolved Hide resolved
Mathlib/SetTheory/Game/PGame.lean Outdated Show resolved Hide resolved
Copy link
Collaborator

@timotree3 timotree3 left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks! I have one question/non-blocking suggestion

Comment on lines +740 to +746
/-- A small family of pre-games is bounded above. -/
lemma bddAbove_range_of_small [Small.{u} ι] (f : ι → PGame.{u}) : BddAbove (Set.range f) := by
let x : PGame.{u} := ⟨Σ i, (f $ (equivShrink.{u} ι).symm i).LeftMoves, PEmpty,
fun x ↦ moveLeft _ x.2, PEmpty.elim⟩
refine ⟨x, Set.forall_range_iff.2 fun i ↦ ?_⟩
rw [← (equivShrink ι).symm_apply_apply i, le_iff_forall_lf]
simpa using fun j ↦ @moveLeft_lf x ⟨equivShrink ι i, j⟩
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a specialization of bddAbove_of_small to small_range. (Although the small_range instance is not currently in scope because we only import Logic.Small.Defs.) Is it really worth exposing as a separate lemma? I suppose the benefit of having it as a seperate lemma is that it makes the proof of bddAbove_of_small more modular. This seems like a tradeoff between API clutter and proof modularity. What is the usual stance on this tradeoff in mathlib? Maybe we can get the best of both worlds by marking this as a private lemma?

Copy link
Collaborator

Choose a reason for hiding this comment

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

On the other hand, maybe this lemma is good for searchability? A user might be expecting to prove BddAbove via a family and not know what Small means so fail to identify that bddAbove_of_small is applicable?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Most predicates in mathlib come in two kinds: A set version and an indexed version. You can define IsFooSet s as IsFooFamily (Subtype.val : s → α) and you can define IsFooFamily f as IsFooSet (Set.range f) (assuming duplicates in your family don't matter). However, it is usually impractical to use one predicate in place of the other:

  • When using IsFooFamily (Subtype.val : s → α), elements of s are now images of the coercion Subtype.val : s → α
  • When using IsFooSet (Set.range f), you need to replace y ∈ Set.range f by f x, x and vice-versa.

It is therefore preferrable to have both predicates (see eg IsDirected). This is currently not the case for BddAbove, where we only have the set version. Hence I am acting as if BddAbove (Set.range _) was a predicate, because many lemmas expect it in that form and because it might become one in the future. The fact that bddAbove_range_of_small can be syntactically derived from bddAbove_of_small is somehow an accident of us not having the family predicate, and I would rather not muddle the waters by only having the set version of the lemma.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Interesting! Thanks for the explanation

Mathlib/SetTheory/Game/PGame.lean Show resolved Hide resolved
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

@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added ready-to-merge This PR has been sent to bors. and removed awaiting-review The author would like community review of the PR labels Feb 28, 2024
mathlib-bors bot pushed a commit that referenced this pull request Feb 28, 2024
@mathlib-bors
Copy link
Contributor

mathlib-bors bot commented Feb 28, 2024

Build failed (retrying...):

  • Build

mathlib-bors bot pushed a commit that referenced this pull request Feb 28, 2024
@mathlib-bors
Copy link
Contributor

mathlib-bors bot commented Feb 28, 2024

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title feat(SetTheory): Upper bound for games [Merged by Bors] - feat(SetTheory): Upper bound for games Feb 28, 2024
@mathlib-bors mathlib-bors bot closed this Feb 28, 2024
@mathlib-bors mathlib-bors bot deleted the YK-Yael-pgame branch February 28, 2024 17:59
riccardobrasca pushed a commit that referenced this pull request Mar 1, 2024
mathlib-bors bot pushed a commit that referenced this pull request Mar 8, 2024
Finish forwarding porting leanprover-community/mathlib#15260 after #10566 ported just the changes in `PGame.lean`.

We don't port `upper_bound_numeric` (or `lower_bound_numeric`) because `upper_bound` was deleted following review feedback on the previous forward-porting PR #10566.



Co-authored-by: Yury G. Kudryashov <urkud@urkud.name>
Co-authored-by: timotree3 <timorcb@gmail.com>
mathlib-bors bot pushed a commit that referenced this pull request Mar 9, 2024
Finish forwarding porting leanprover-community/mathlib#15260 after #10566 ported just the changes in `PGame.lean`.

We don't port `upper_bound_numeric` (or `lower_bound_numeric`) because `upper_bound` was deleted following review feedback on the previous forward-porting PR #10566.



Co-authored-by: Yury G. Kudryashov <urkud@urkud.name>
Co-authored-by: timotree3 <timorcb@gmail.com>
mathlib-bors bot pushed a commit that referenced this pull request Mar 9, 2024
Finish forwarding porting leanprover-community/mathlib#15260 after #10566 ported just the changes in `PGame.lean`.

We don't port `upper_bound_numeric` (or `lower_bound_numeric`) because `upper_bound` was deleted following review feedback on the previous forward-porting PR #10566.



Co-authored-by: Yury G. Kudryashov <urkud@urkud.name>
Co-authored-by: timotree3 <timorcb@gmail.com>
kbuzzard pushed a commit that referenced this pull request Mar 12, 2024
kbuzzard pushed a commit that referenced this pull request Mar 12, 2024
Finish forwarding porting leanprover-community/mathlib#15260 after #10566 ported just the changes in `PGame.lean`.

We don't port `upper_bound_numeric` (or `lower_bound_numeric`) because `upper_bound` was deleted following review feedback on the previous forward-porting PR #10566.



Co-authored-by: Yury G. Kudryashov <urkud@urkud.name>
Co-authored-by: timotree3 <timorcb@gmail.com>
dagurtomas pushed a commit that referenced this pull request Mar 22, 2024
dagurtomas pushed a commit that referenced this pull request Mar 22, 2024
Finish forwarding porting leanprover-community/mathlib#15260 after #10566 ported just the changes in `PGame.lean`.

We don't port `upper_bound_numeric` (or `lower_bound_numeric`) because `upper_bound` was deleted following review feedback on the previous forward-porting PR #10566.



Co-authored-by: Yury G. Kudryashov <urkud@urkud.name>
Co-authored-by: timotree3 <timorcb@gmail.com>
utensil pushed a commit that referenced this pull request Mar 26, 2024
Finish forwarding porting leanprover-community/mathlib#15260 after #10566 ported just the changes in `PGame.lean`.

We don't port `upper_bound_numeric` (or `lower_bound_numeric`) because `upper_bound` was deleted following review feedback on the previous forward-porting PR #10566.



Co-authored-by: Yury G. Kudryashov <urkud@urkud.name>
Co-authored-by: timotree3 <timorcb@gmail.com>
Louddy pushed a commit that referenced this pull request Apr 15, 2024
Louddy pushed a commit that referenced this pull request Apr 15, 2024
Finish forwarding porting leanprover-community/mathlib#15260 after #10566 ported just the changes in `PGame.lean`.

We don't port `upper_bound_numeric` (or `lower_bound_numeric`) because `upper_bound` was deleted following review feedback on the previous forward-porting PR #10566.



Co-authored-by: Yury G. Kudryashov <urkud@urkud.name>
Co-authored-by: timotree3 <timorcb@gmail.com>
uniwuni pushed a commit that referenced this pull request Apr 19, 2024
Finish forwarding porting leanprover-community/mathlib#15260 after #10566 ported just the changes in `PGame.lean`.

We don't port `upper_bound_numeric` (or `lower_bound_numeric`) because `upper_bound` was deleted following review feedback on the previous forward-porting PR #10566.



Co-authored-by: Yury G. Kudryashov <urkud@urkud.name>
Co-authored-by: timotree3 <timorcb@gmail.com>
callesonne pushed a commit that referenced this pull request Apr 22, 2024
Finish forwarding porting leanprover-community/mathlib#15260 after #10566 ported just the changes in `PGame.lean`.

We don't port `upper_bound_numeric` (or `lower_bound_numeric`) because `upper_bound` was deleted following review feedback on the previous forward-porting PR #10566.



Co-authored-by: Yury G. Kudryashov <urkud@urkud.name>
Co-authored-by: timotree3 <timorcb@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mathlib3-pair This PR is a forward-port of a mathlib3 PR or part of one, either under review or recently merged 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

4 participants