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

feat: swap arguments to Membership.mem #5020

Merged
merged 5 commits into from
Aug 26, 2024

Conversation

mattrobball
Copy link
Contributor

@mattrobball mattrobball commented Aug 13, 2024

We swap the arguments for Membership.mem so that when proceeded by a SetLike coercion, as is often the case in Mathlib, the resulting expression is recognized as eta expanded and reduce for many computations. The most beneficial outcome is that the discrimination tree keys for instances and simp lemmas concerning subsets become more robust resulting in more efficient searches.

Closes RFC #4932

@github-actions github-actions bot added the toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN label Aug 13, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Aug 13, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Aug 13, 2024
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added breaks-mathlib This is not necessarily a blocker for merging: but there needs to be a plan release-ci Enable all CI checks for a PR, like is done for releases labels Aug 13, 2024
@leanprover-community-mathlib4-bot
Copy link
Collaborator

leanprover-community-mathlib4-bot commented Aug 13, 2024

Mathlib CI status (docs):

@mattrobball
Copy link
Contributor Author

mattrobball commented Aug 15, 2024

I should have clean CI for the PR branch of mathlib and all but one core test passing. I will try to fix that later tonight and highlight the less-than-desirable changes.

@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added builds-mathlib CI has verified that Mathlib builds against this PR and removed breaks-mathlib This is not necessarily a blocker for merging: but there needs to be a plan labels Aug 15, 2024
@mattrobball
Copy link
Contributor Author

awaiting-review

leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Aug 16, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Aug 16, 2024
@mattrobball
Copy link
Contributor Author

@TwoFX and @semorrison I attempted to apply the label for awaiting review but it failed.

There is now a working branch of mathlib. I created leanprover-community/mathlib4#15870 to, at the moment, provide an interface to highlight anything surprising.

@leanprover-bot leanprover-bot added the P-medium We may work on this issue if we find the time label Aug 16, 2024
@kim-em kim-em force-pushed the mrb/mem_swap branch 2 times, most recently from af06c79 to 88ce999 Compare August 23, 2024 03:54
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Aug 23, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Aug 23, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Aug 24, 2024
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added breaks-mathlib This is not necessarily a blocker for merging: but there needs to be a plan and removed builds-mathlib CI has verified that Mathlib builds against this PR labels Aug 26, 2024
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added builds-mathlib CI has verified that Mathlib builds against this PR and removed breaks-mathlib This is not necessarily a blocker for merging: but there needs to be a plan labels Aug 26, 2024
@TwoFX TwoFX added this pull request to the merge queue Aug 26, 2024
Merged via the queue into leanprover:master with commit b54a9ec Aug 26, 2024
25 checks passed
mathlib-bors bot pushed a commit to leanprover-community/mathlib4 that referenced this pull request Sep 5, 2024
Currently we use `Set.inclusion` to construct functions between `Opens`. This is a form of defeq abuse which is exposed by leanprover/lean4#5020. In this PR, we introduce `Opens.inclusion` and replace uses of `Set.inclusion` with it to avoid passing through a coercion to `Set`. Since there is a version, not analogous to `Set.inclusion`, squatting on this name we rename it to `Opens.inclusion'`.



Co-authored-by: Matthew Robert Ballard <[email protected]>
mathlib-bors bot pushed a commit to leanprover-community/mathlib4 that referenced this pull request Sep 6, 2024
We make miscellaneous changes to either remove or clarify `adaptation_notes` coming from leanprover/lean4#5020.



Co-authored-by: Kim Morrison <[email protected]>
bjoernkjoshanssen pushed a commit to leanprover-community/mathlib4 that referenced this pull request Sep 9, 2024
Currently we use `Set.inclusion` to construct functions between `Opens`. This is a form of defeq abuse which is exposed by leanprover/lean4#5020. In this PR, we introduce `Opens.inclusion` and replace uses of `Set.inclusion` with it to avoid passing through a coercion to `Set`. Since there is a version, not analogous to `Set.inclusion`, squatting on this name we rename it to `Opens.inclusion'`.



Co-authored-by: Matthew Robert Ballard <[email protected]>
bjoernkjoshanssen pushed a commit to leanprover-community/mathlib4 that referenced this pull request Sep 9, 2024
We make miscellaneous changes to either remove or clarify `adaptation_notes` coming from leanprover/lean4#5020.



Co-authored-by: Kim Morrison <[email protected]>
bjoernkjoshanssen pushed a commit to leanprover-community/mathlib4 that referenced this pull request Sep 9, 2024
Currently we use `Set.inclusion` to construct functions between `Opens`. This is a form of defeq abuse which is exposed by leanprover/lean4#5020. In this PR, we introduce `Opens.inclusion` and replace uses of `Set.inclusion` with it to avoid passing through a coercion to `Set`. Since there is a version, not analogous to `Set.inclusion`, squatting on this name we rename it to `Opens.inclusion'`.



Co-authored-by: Matthew Robert Ballard <[email protected]>
bjoernkjoshanssen pushed a commit to leanprover-community/mathlib4 that referenced this pull request Sep 9, 2024
We make miscellaneous changes to either remove or clarify `adaptation_notes` coming from leanprover/lean4#5020.



Co-authored-by: Kim Morrison <[email protected]>
bjoernkjoshanssen pushed a commit to leanprover-community/mathlib4 that referenced this pull request Sep 12, 2024
Currently we use `Set.inclusion` to construct functions between `Opens`. This is a form of defeq abuse which is exposed by leanprover/lean4#5020. In this PR, we introduce `Opens.inclusion` and replace uses of `Set.inclusion` with it to avoid passing through a coercion to `Set`. Since there is a version, not analogous to `Set.inclusion`, squatting on this name we rename it to `Opens.inclusion'`.



Co-authored-by: Matthew Robert Ballard <[email protected]>
bjoernkjoshanssen pushed a commit to leanprover-community/mathlib4 that referenced this pull request Sep 12, 2024
We make miscellaneous changes to either remove or clarify `adaptation_notes` coming from leanprover/lean4#5020.



Co-authored-by: Kim Morrison <[email protected]>
mathlib-bors bot pushed a commit to leanprover-community/mathlib4 that referenced this pull request Sep 24, 2024
…dule` to get a type (#16509)

We add
```lean
instance (priority := high) coeSort : CoeSort (LieSubmodule R L M) (Type w) where
  coe N := { x : M // x ∈ N }
```
to avoid the coercion to a `Sort` being `{ x : M // x ∈ N.toSubmodule }` and instead be `N` itself.

This allows removal of all `adapation_note`s from leanprover/lean4#5020.

Ultimately the API here should be expanded and `Coe (LieSubmodule R L M) (Submodule R M)`
should be minimized or removed to avoid forcing Lean to unify `LieSubmodule` and `Submodule` unnecessarily.



Co-authored-by: Matthew Robert Ballard <[email protected]>
mathlib-bors bot pushed a commit to leanprover-community/mathlib4 that referenced this pull request Sep 27, 2024
fbarroero pushed a commit to leanprover-community/mathlib4 that referenced this pull request Oct 2, 2024
nahcnuj added a commit to nahcnuj/hello-type-system that referenced this pull request Oct 10, 2024
The parameters to Membership.mem have been swapped, which affects all Membership instances.

leanprover/lean4#5020
nomeata added a commit that referenced this pull request Nov 11, 2024
This PR fixes `simp only [· ∈ ·]` after #5020.

Fixes #5905
github-merge-queue bot pushed a commit that referenced this pull request Nov 11, 2024
This PR fixes `simp only [· ∈ ·]` after #5020.

Fixes #5905
JovanGerb pushed a commit to JovanGerb/lean4 that referenced this pull request Jan 21, 2025
This PR fixes `simp only [· ∈ ·]` after leanprover#5020.

Fixes leanprover#5905
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
builds-mathlib CI has verified that Mathlib builds against this PR P-medium We may work on this issue if we find the time release-ci Enable all CI checks for a PR, like is done for releases toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants