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(data/fin): add_comm_monoid and simp lemmas #5010

Closed
wants to merge 7 commits into from

Conversation

pechersky
Copy link
Collaborator

@pechersky pechersky commented Nov 15, 2020

Provide add_comm_monoid for fin (n + 1), which makes proofs that have to do with bit0, bit1, and nat.cast and related happy. Provide the specialized lemmas as simp lemmas. Also provide various simp lemmas about multiplication, but without the associated comm_monoid.


I didn't make the comm_monoid for multiplication, partially because I don't have a direct need for it, and partially because I couldn't juggle the mul_assoc proof out of the modular arithmetic nor backport from the zmod file.

New lemmas:
`add_zero`
`zero_add`
`mul_one`
`one_mul`
@pechersky pechersky added awaiting-review The author would like community review of the PR easy < 20s of review time. See the lifecycle page for guidelines. awaiting-author A reviewer has asked the author a question or requested changes and removed awaiting-review The author would like community review of the PR easy < 20s of review time. See the lifecycle page for guidelines. labels Nov 15, 2020
@pechersky
Copy link
Collaborator Author

I guess the question is, whether these lemmas should be available in data/fin even without the instances provided in data/zmod/basic.

@pechersky pechersky added the WIP Work in progress label Nov 15, 2020
@github-actions github-actions bot added the merge-conflict Please `git merge origin/master` then a bot will remove this label. label Dec 12, 2020
@github-actions github-actions bot removed the merge-conflict Please `git merge origin/master` then a bot will remove this label. label Jan 13, 2021
@pechersky pechersky changed the title feat(data/fin): identities in add and mul monoids feat(data/fin): add_comm_monoid and simp lemmas Jan 13, 2021
@pechersky pechersky added awaiting-review The author would like community review of the PR and removed WIP Work in progress awaiting-author A reviewer has asked the author a question or requested changes labels Jan 14, 2021
@robertylewis
Copy link
Member

I know there's some controversy about arithmetic operations on fin n:

This PR doesn't add to the controversy, and the simp lemmas clean up the situation slightly. (Although you could argue that cleaning things up encourages people to use the controversial operations...) I think it's fine to merge this until there's a better story about fin.

bors merge

@github-actions github-actions bot added ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.) and removed awaiting-review The author would like community review of the PR labels Jan 20, 2021
bors bot pushed a commit that referenced this pull request Jan 20, 2021
Provide `add_comm_monoid` for `fin (n + 1)`, which makes proofs that have to do with `bit0`, `bit1`, and `nat.cast` and related happy. Provide the specialized lemmas as simp lemmas. Also provide various simp lemmas about multiplication, but without the associated `comm_monoid`.
@bors
Copy link

bors bot commented Jan 20, 2021

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title feat(data/fin): add_comm_monoid and simp lemmas [Merged by Bors] - feat(data/fin): add_comm_monoid and simp lemmas Jan 20, 2021
@bors bors bot closed this Jan 20, 2021
@bors bors bot deleted the fin-monoid-identities branch January 20, 2021 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants