Skip to content

[Merged by Bors] - feat(SetTheory): Upper bound for games#10566

Closed
YaelDillies wants to merge 2 commits intomasterfrom
YK-Yael-pgame
Closed

[Merged by Bors] - feat(SetTheory): Upper bound for games#10566
YaelDillies wants to merge 2 commits intomasterfrom
YK-Yael-pgame

Conversation

@YaelDillies
Copy link
Copy Markdown
Contributor

Partially forward-port leanprover-community/mathlib3#15260


Open in Gitpod

@YaelDillies YaelDillies added awaiting-review 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, 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
Copy Markdown
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/mathlib3#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.

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor 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
Copy Markdown
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

Copy link
Copy Markdown
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

@ghost ghost added ready-to-merge This PR has been sent to bors. and removed awaiting-review labels Feb 28, 2024
mathlib-bors bot pushed a commit that referenced this pull request Feb 28, 2024
@mathlib-bors
Copy link
Copy Markdown
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
Copy Markdown
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/mathlib3#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/mathlib3#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/mathlib3#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/mathlib3#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/mathlib3#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/mathlib3#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>
xgenereux pushed a commit that referenced this pull request Apr 15, 2024
xgenereux pushed a commit that referenced this pull request Apr 15, 2024
Finish forwarding porting leanprover-community/mathlib3#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/mathlib3#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/mathlib3#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, etc)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants