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 Algebra.BigOperators.Fin #1848

Closed
wants to merge 35 commits into from

Conversation

joneugster
Copy link
Collaborator

@joneugster joneugster commented Jan 26, 2023

@joneugster joneugster added WIP Work in progress mathlib-port This is a port of a theory file from mathlib. labels Jan 26, 2023
@semorrison semorrison added the blocked-by-other-PR This PR depends on another PR which is still in the queue. label Jan 26, 2023
@semorrison semorrison added merge-conflict The PR has a merge conflict with master, and needs manual merging. and removed blocked-by-other-PR This PR depends on another PR which is still in the queue. labels Jan 26, 2023
@semorrison semorrison added the blocked-by-other-PR This PR depends on another PR which is still in the queue. label Jan 26, 2023
@semorrison semorrison removed the merge-conflict The PR has a merge conflict with master, and needs manual merging. label Jan 26, 2023
theorem partialProd_right_inv {G : Type _} [Group G] (g : G) (f : Fin n → G) (i : Fin n) :
((g • partialProd f) (Fin.castLt i (Nat.lt_succ_of_lt i.2)))⁻¹ *
(g • partialProd f) i.succ = f i := by
rcases i with ⟨i, hn⟩
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This theorem was a nightmare to prove. Lots of things that should be def-eq aren't recoginsed as such by rw (e.g. n + 1 vs n.succ). Moreover assoc_rw would be really helpful.

Could somebody look at the proof and decide if there is anything that could be done to avoid non-terminal simp and all the change?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Also the statement has changed. In Lean3 it was ↑i instead of (Fin.castLt i (Nat.lt_succ_of_lt i.2)). I changed that because Lean4 got the conversion wrong, could that be fixed too?

Copy link
Contributor

Choose a reason for hiding this comment

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

I see, Lean 4 coerces via the path Fin n → ℕ → Fin (n + 1) which results in different defeqs than castLt. Although I can't actually find where the coercion comes from in mathlib 3...

Copy link
Contributor

Choose a reason for hiding this comment

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

Nope, turns out that Lean 3 finds the same coercion (@coe_trans (fin n) ℕ (fin (n + 1)) nat.cast_coe (fin.fin_to_nat n)). So I think it's best to keep the statement with a coercion.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for writing the extensive porting note.
Let's merge this PR now. We can revisit this issue later.

@joneugster joneugster added awaiting-review The author would like community review of the PR and removed WIP Work in progress labels Jan 27, 2023
@jcommelin jcommelin added awaiting-author A reviewer has asked the author a question or requested changes awaiting-review The author would like community review of the PR and removed awaiting-review The author would like community review of the PR awaiting-author A reviewer has asked the author a question or requested changes labels Feb 1, 2023
@jcommelin
Copy link
Member

Thanks 🎉

bors merge

@bors
Copy link

bors bot commented Feb 1, 2023

👎 Rejected by label

@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 Feb 1, 2023
@jcommelin jcommelin removed the blocked-by-other-PR This PR depends on another PR which is still in the queue. label Feb 1, 2023
@jcommelin
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Feb 1, 2023
Co-authored-by: Floris van Doorn <fpvdoorn@gmail.com>
@semorrison semorrison added the blocked-by-other-PR This PR depends on another PR which is still in the queue. label Feb 1, 2023
@jcommelin jcommelin removed the blocked-by-other-PR This PR depends on another PR which is still in the queue. label Feb 1, 2023
@semorrison
Copy link
Contributor

This PR/issue depends on:

@bors
Copy link

bors bot commented Feb 1, 2023

Build failed:

  • Build

@jcommelin
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Feb 1, 2023
Co-authored-by: Floris van Doorn <fpvdoorn@gmail.com>
@bors
Copy link

bors bot commented Feb 1, 2023

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title feat: port Algebra.BigOperators.Fin [Merged by Bors] - feat: port Algebra.BigOperators.Fin Feb 1, 2023
@bors bors bot closed this Feb 1, 2023
@bors bors bot deleted the port/Algebra.BigOperators.Fin branch February 1, 2023 16:56
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

5 participants