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 LinearAlgebra.CliffordAlgebra.Grading #5349

Closed

Conversation

eric-wieser
Copy link
Member

@eric-wieser eric-wieser commented Jun 21, 2023

This runs into some annoying problems with leanprover/lean4#1926 which makes working with the dependent types much worse than it was in Lean 3.


Open in Gitpod

@eric-wieser eric-wieser added the mathlib-port This is a port of a theory file from mathlib. label Jun 21, 2023
@kim-em kim-em added blocked-by-other-PR This PR depends on another PR to Mathlib (this label is automatically managed by a bot) merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) labels Jun 21, 2023
Comment on lines 182 to 183
-- TODO: this `simp_rw` is not actually doing anything, lean4#1926
simp_rw [Subtype.coe_mk, ZMod.nat_coe_zmod_eq_iff, add_comm n.val]
Copy link
Member Author

@eric-wieser eric-wieser Jun 21, 2023

Choose a reason for hiding this comment

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

leanprover/lean4#1926. The proof fails without the effect this line is supposed to have.

@eric-wieser eric-wieser added the help-wanted The author needs attention to resolve issues label Jun 21, 2023
@kim-em kim-em removed merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) blocked-by-other-PR This PR depends on another PR to Mathlib (this label is automatically managed by a bot) labels Jun 21, 2023
@kim-em
Copy link
Contributor

kim-em commented Jun 22, 2023

This PR/issue depends on:

@kim-em kim-em added the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Jun 22, 2023
Mathbin -> Mathlib
fix certain import statements
move "by" to end of line
add import to Mathlib.lean
@eric-wieser eric-wieser force-pushed the port/LinearAlgebra.CliffordAlgebra.Grading branch from dc23f95 to 3ced1df Compare June 22, 2023 23:31
@kim-em kim-em removed the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Jun 22, 2023
@eric-wieser eric-wieser self-assigned this Jun 22, 2023
@eric-wieser eric-wieser added awaiting-review and removed help-wanted The author needs attention to resolve issues labels Jun 22, 2023
Comment on lines 170 to 192
simp_rw [Subtype.coe_mk, ZMod.nat_coe_zmod_eq_iff, add_comm n.val]
-- porting note: was `simp_rw [Subtype.coe_mk, ZMod.nat_coe_zmod_eq_iff, add_comm n.val]`
-- leanprover/lean4#1926 is to blame.
dsimp only [Subtype.coe_mk]
let hlean1926 : ∀ val : ℕ, ↑val = n ↔ ∃ k, val = 2 * k + ZMod.val n := by
intro val
simp_rw [ZMod.nat_coe_zmod_eq_iff, add_comm n.val]
have := fun val : ℕ => forall_prop_congr'
(q := fun property => ∀ x (hx : x ∈ LinearMap.range (ι Q) ^ val),
P x (Submodule.mem_iSup_of_mem { val := val, property := property } hx))
(hq := fun property => Iff.rfl) (hp := hlean1926 val)
simp_rw [this]; clear this
-- end porting note
Copy link
Member Author

Choose a reason for hiding this comment

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

This is extremely unfortunate, and is a major regression in simp's ability to deal with dependent types.

Comment on lines +215 to +217
let hlean1926'' : x ∈ LinearMap.range (ι Q) ^ 2
↔ x ∈ LinearMap.range (ι Q) * LinearMap.range (ι Q) := by
rw [pow_two]
Copy link
Member Author

@eric-wieser eric-wieser Jun 22, 2023

Choose a reason for hiding this comment

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

Interestingly the proof fails if I use a have instead of a let, as then the induction motive below is wrong.

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 for carefully documenting the regressions. Let's hope they get fixed soon!

bors merge

@kim-em kim-em added ready-to-merge This PR has been sent to bors. and removed awaiting-review labels Jun 23, 2023
bors bot pushed a commit that referenced this pull request Jun 23, 2023
This runs into some annoying problems with leanprover/lean4#1926 which makes working with the dependent types much worse than it was in Lean 3.
@bors
Copy link

bors bot commented Jun 23, 2023

Pull request successfully merged into master.

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot changed the title feat: port LinearAlgebra.CliffordAlgebra.Grading [Merged by Bors] - feat: port LinearAlgebra.CliffordAlgebra.Grading Jun 23, 2023
@bors bors bot closed this Jun 23, 2023
@bors bors bot deleted the port/LinearAlgebra.CliffordAlgebra.Grading branch June 23, 2023 04:35
kim-em pushed a commit that referenced this pull request Jun 23, 2023
This runs into some annoying problems with leanprover/lean4#1926 which makes working with the dependent types much worse than it was in Lean 3.
kim-em pushed a commit that referenced this pull request Jun 23, 2023
This runs into some annoying problems with leanprover/lean4#1926 which makes working with the dependent types much worse than it was in Lean 3.
kim-em pushed a commit that referenced this pull request Jun 25, 2023
This runs into some annoying problems with leanprover/lean4#1926 which makes working with the dependent types much worse than it was in Lean 3.
kbuzzard pushed a commit that referenced this pull request Jul 6, 2023
This runs into some annoying problems with leanprover/lean4#1926 which makes working with the dependent types much worse than it was in Lean 3.
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.

3 participants