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: port Data.Set.Basic #892

Closed
wants to merge 36 commits into from
Closed

Conversation

semorrison
Copy link
Contributor

@semorrison semorrison commented Dec 6, 2022

1b36dabc50929b36caec16306358a5cc44ab441e

If more PRs arrive in mathlib that slice up Data.Set.Basic, that's okay, we'll just delete the relevant sections here.

TODO: finish linting

@semorrison semorrison added help-wanted The author needs attention to resolve issues WIP Work in progress labels Dec 7, 2022
@semorrison semorrison changed the title initial look at data.set.basic (pre-mathlib3 refactors) feat: port Data.Set.Basic Dec 7, 2022
@Ruben-VandeVelde Ruben-VandeVelde added the mathlib-port This is a port of a theory file from mathlib. label Dec 8, 2022
@semorrison semorrison added awaiting-review The author would like community review of the PR and removed help-wanted The author needs attention to resolve issues WIP Work in progress labels Dec 8, 2022
Mathlib/Data/Set/Basic.lean Outdated Show resolved Hide resolved
Mathlib/Data/Set/Basic.lean Outdated Show resolved Hide resolved
Mathlib/Data/Set/Basic.lean Outdated Show resolved Hide resolved
Mathlib/Data/Set/Basic.lean Outdated Show resolved Hide resolved
Mathlib/Data/Set/Basic.lean Outdated Show resolved Hide resolved
Mathlib/Data/Set/Basic.lean Outdated Show resolved Hide resolved
Mathlib/Data/Set/Basic.lean Outdated Show resolved Hide resolved
@Ruben-VandeVelde
Copy link
Collaborator

Lint failures:

./build/bin/runLinter
-- Found 2 errors in 15315 declarations (plus 44200 automatically generated ones) in mathlib with 13 linters

/- The `simpNF` linter reports:
SOME SIMP LEMMAS ARE NOT IN SIMP-NORMAL FORM.
see note [simp-normal form] for tips how to debug this.
https://leanprover-community.github.io/mathlib_docs/notes.html#simp-normal%20form -/
-- Mathlib.Data.Set.Basic
#check Set.sep_union.{u} /- Left-hand side simplifies from
  { x | x ∈ s ∪ t ∧ p x }
to
  { x | (x ∈ s ∨ x ∈ t) ∧ p x }
using
  simp only [Set.mem_union]
Try to change the left-hand side to the simplified term!
 -/
#check Set.sep_inter.{u} /- Left-hand side simplifies from
  { x | x ∈ s ∩ t ∧ p x }
to
  { x | (x ∈ s ∧ x ∈ t) ∧ p x }
using
  simp only [Set.mem_inter_iff]
Try to change the left-hand side to the simplified term!
 -/

@eric-wieser
Copy link
Member

It looks like this is because set.sep is now unfolded syntactically like coe?

@[simp, norm_cast]
theorem nontrivial_coe_sort {s : Set α} : Nontrivial s ↔ s.Nontrivial := by
-- simp_rw [← nontrivial_univ_iff, Set.Nontrivial, mem_univ, exists_true_left, SetCoe.exists,
-- Subtype.mk_eq_mk]
Copy link
Collaborator

Choose a reason for hiding this comment

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

simp_rw [th] in mathlib3 can rewrite ∃ h : p, ..., where p can be proven using theorem th, into ∃ h : true, ..., but is unable to do so in mathlib4.

Tangentially, the pretty printer also doesn't properly render ∃ h : p, ... but instead as Exists fun h ↦ ..., so the type of h is somewhat obscured.

Copy link
Member

@kbuzzard kbuzzard Dec 11, 2022

Choose a reason for hiding this comment

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

People are aware about the prettyprinter issue with Exists, which was caused by the introduction of the notation; at the porting meeting the idea was discussed about reverting the notation change or doing it "properly" (which would involve changes in core Lean).

I've also seen other discussions about the simplifier behaving differently, including this issue; it is hopefully covered by leanprover/lean4#1937 .

Copy link
Collaborator

@Ruben-VandeVelde Ruben-VandeVelde left a comment

Choose a reason for hiding this comment

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

I think this can probably land now

@semorrison
Copy link
Contributor Author

bors merge

@github-actions github-actions 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 Dec 12, 2022
@semorrison
Copy link
Contributor Author

bors merge

bors bot pushed a commit that referenced this pull request Dec 12, 2022
1b36dabc50929b36caec16306358a5cc44ab441e

If more PRs arrive in mathlib that slice up Data.Set.Basic, that's okay, we'll just delete the relevant sections here.

TODO: finish linting


Co-authored-by: Scott Morrison <scott.morrison@gmail.com>
Co-authored-by: ChrisHughes24 <chrishughes24@gmail.com>
Co-authored-by: Chris Hughes <33847686+ChrisHughes24@users.noreply.github.com>
Co-authored-by: Winston Yin <winstonyin@gmail.com>
Co-authored-by: Ruben Van de Velde <65514131+Ruben-VandeVelde@users.noreply.github.com>
@bors
Copy link

bors bot commented Dec 12, 2022

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title feat: port Data.Set.Basic [Merged by Bors] - feat: port Data.Set.Basic Dec 12, 2022
@bors bors bot closed this Dec 12, 2022
@bors bors bot deleted the data_set_basic branch December 12, 2022 01:46
bors bot pushed a commit that referenced this pull request Dec 12, 2022
mathlib3 SHA: f178c0e25af359f6cbc72a96a243efd3b12423a3

- [x] depends on: #892

Co-authored-by: ChrisHughes24 <chrishughes24@gmail.com>
Co-authored-by: Chris Hughes <33847686+ChrisHughes24@users.noreply.github.com>
Co-authored-by: Winston Yin <winstonyin@gmail.com>
bors bot pushed a commit that referenced this pull request Dec 12, 2022
mathlib3 SHA: 10b4e499f43088dd3bb7b5796184ad5216648ab1

- [x] Depends on #892
- [x] Depends on #907

Co-authored-by: ChrisHughes24 <chrishughes24@gmail.com>
bors bot pushed a commit that referenced this pull request Dec 12, 2022
mathlib3 SHA: 10b4e499f43088dd3bb7b5796184ad5216648ab1

- [x] Depends on #892
- [x] Depends on #907

Co-authored-by: ChrisHughes24 <chrishughes24@gmail.com>
bors bot pushed a commit to leanprover-community/mathlib that referenced this pull request Dec 15, 2022
Regenerated from the [port status wiki page](https://github.com/leanprover-community/mathlib/wiki/mathlib4-port-status).
Relates to the following PRs:
* `algebra.euclidean_domain.basic`: leanprover-community/mathlib4#919
* `algebra.field.basic`: leanprover-community/mathlib4#975
* `algebra.group.opposite`: leanprover-community/mathlib4#912
* `algebra.group.prod`: leanprover-community/mathlib4#968
* `algebra.group.with_one.units`: leanprover-community/mathlib4#955
* `algebra.group_power.ring`: leanprover-community/mathlib4#979
* `algebra.hom.equiv.type_tags`: leanprover-community/mathlib4#943
* `algebra.hom.ring`: leanprover-community/mathlib4#958
* `algebra.invertible`: leanprover-community/mathlib4#930
* `algebra.order.field.canonical.basic`: leanprover-community/mathlib4#1018
* `algebra.order.field.canonical.defs`: leanprover-community/mathlib4#985
* `algebra.order.field.inj_surj`: leanprover-community/mathlib4#1017
* `algebra.order.group.densely_ordered`: leanprover-community/mathlib4#956
* `algebra.order.group.min_max`: leanprover-community/mathlib4#933
* `algebra.order.group.prod`: leanprover-community/mathlib4#1026
* `algebra.order.group.with_top`: leanprover-community/mathlib4#946
* `algebra.order.hom.monoid`: leanprover-community/mathlib4#944
* `algebra.order.monoid.prod`: leanprover-community/mathlib4#987
* `algebra.order.monoid.to_mul_bot`: leanprover-community/mathlib4#1024
* `algebra.order.ring.abs`: leanprover-community/mathlib4#929
* `algebra.order.ring.cone`: leanprover-community/mathlib4#935
* `algebra.order.sub.basic`: leanprover-community/mathlib4#936
* `algebra.order.sub.with_top`: leanprover-community/mathlib4#932
* `algebra.order.with_zero`: leanprover-community/mathlib4#903
* `combinatorics.quiver.arborescence`: leanprover-community/mathlib4#997
* `combinatorics.quiver.cast`: leanprover-community/mathlib4#990
* `control.traversable.lemmas`: leanprover-community/mathlib4#948
* `data.bool.set`: leanprover-community/mathlib4#960
* `data.int.cast.field`: leanprover-community/mathlib4#1016
* `data.int.cast.lemmas`: leanprover-community/mathlib4#995
* `data.int.cast.prod`: leanprover-community/mathlib4#1015
* `data.int.div`: leanprover-community/mathlib4#1011
* `data.int.dvd.basic`: leanprover-community/mathlib4#996
* `data.int.order.basic`: leanprover-community/mathlib4#938
* `data.nat.cast.basic`: leanprover-community/mathlib4#980
* `data.nat.cast.prod`: leanprover-community/mathlib4#1010
* `data.nat.cast.with_top`: leanprover-community/mathlib4#1019
* `data.nat.gcd.basic`: leanprover-community/mathlib4#965
* `data.nat.order.basic`: leanprover-community/mathlib4#907
* `data.nat.order.lemmas`: leanprover-community/mathlib4#927
* `data.nat.set`: leanprover-community/mathlib4#961
* `data.nat.upto`: leanprover-community/mathlib4#1020
* `data.pequiv`: leanprover-community/mathlib4#1008
* `data.set.basic`: leanprover-community/mathlib4#892
* `data.set.bool_indicator`: leanprover-community/mathlib4#988
* `data.set.image`: leanprover-community/mathlib4#949
* `data.set.n_ary`: leanprover-community/mathlib4#969
* `data.set.opposite`: leanprover-community/mathlib4#983
* `data.set.prod`: leanprover-community/mathlib4#1004
* `data.set.sigma`: leanprover-community/mathlib4#982
* `data.set_like.basic`: leanprover-community/mathlib4#993
* `data.two_pointing`: leanprover-community/mathlib4#984
* `logic.embedding.set`: leanprover-community/mathlib4#986
* `logic.equiv.embedding`: leanprover-community/mathlib4#1021
* `order.directed`: leanprover-community/mathlib4#963
* `order.rel_iso.set`: leanprover-community/mathlib4#1005
* `order.well_founded`: leanprover-community/mathlib4#970
* `ring_theory.coprime.basic`: leanprover-community/mathlib4#899



Co-authored-by: Jireh Loreaux <loreaujy@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mathlib-port This is a port of a theory file from mathlib. ready-to-merge This PR has been sent to bors.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants