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(algebra/group/with_one): more API for with_zero #3716

Closed
wants to merge 36 commits into from

Conversation

kbuzzard
Copy link
Member

@kbuzzard kbuzzard commented Aug 7, 2020


I need this to prove that with_zero (multiplicative int) is a linear_ordered_comm_group_with_zero.

@kbuzzard kbuzzard added the awaiting-review The author would like community review of the PR label Aug 7, 2020
@bryangingechen bryangingechen changed the title feat(with_zero) : more API for with_zero feat(algebra/group/with_one): more API for with_zero Aug 7, 2020
@kbuzzard kbuzzard removed the awaiting-review The author would like community review of the PR label Aug 7, 2020
@semorrison semorrison added the WIP Work in progress label Aug 12, 2020
@semorrison
Copy link
Collaborator

Labelling as WIP while it's not building.

@kbuzzard
Copy link
Member Author

I am being trolled by the linter. This PR is nearly ready except that the linter complained here that with_one.coe_mul wasn't a good simp lemma because it could be proved using with_zero.coe_mul. I am not sure I believe the linter here.

@jcommelin
Copy link
Member

jcommelin commented Aug 18, 2020

@kbuzzard Try proving with_zero.coe_mul using with_one.coe_mul _ _. It works.
But by simp only [with_one.coe_mul] will not prove it.
However, click on the up arrows in the goal state of

@[simp, norm_cast] lemma coe_mul {α : Type u} [has_mul α]
  {a b : α} : ((a * b : α) : with_zero α) = a * b :=
by { simp, /- cursor here -/ }

simp managed to mix with_zero and with_one. This seems bad.

Putting

local attribute [irreducible] with_zero with_one

at the bottom of the file, before the #lint the complaints.
Maybe, if the API is solid, you should mark with_zero and with_one as irreducible.

@kbuzzard kbuzzard added awaiting-review The author would like community review of the PR and removed WIP Work in progress labels Aug 24, 2020
@kbuzzard
Copy link
Member Author

OK this is ready for review. The main result is that if G is an ordered_comm_monoid then so is with_zero G; the instance was not there. I add some simp lemmas too.

Copy link
Member

@fpvandoorn fpvandoorn left a comment

Choose a reason for hiding this comment

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

LGTM, here are some minor suggestions.

src/algebra/ordered_group.lean Outdated Show resolved Hide resolved
src/algebra/ordered_group.lean Outdated Show resolved Hide resolved
src/algebra/ordered_group.lean Outdated Show resolved Hide resolved
src/algebra/ordered_group.lean Outdated Show resolved Hide resolved
@semorrison semorrison added 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 labels Aug 25, 2020
@kbuzzard
Copy link
Member Author

Thanks for these @fpvandoorn

@kbuzzard kbuzzard added awaiting-review The author would like community review of the PR and removed awaiting-author A reviewer has asked the author a question or requested changes labels Aug 25, 2020
Copy link
Member

@fpvandoorn fpvandoorn left a comment

Choose a reason for hiding this comment

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

LGTM

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 Aug 25, 2020
@bors
Copy link

bors bot commented Aug 25, 2020

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title feat(algebra/group/with_one): more API for with_zero [Merged by Bors] - feat(algebra/group/with_one): more API for with_zero Aug 25, 2020
@bors bors bot closed this Aug 25, 2020
@bors bors bot deleted the with-zero-stuff branch August 25, 2020 21:15
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

4 participants