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(group_theory/exponent): exponent G = ⨆ g : G, order_of g #10767

Closed
wants to merge 30 commits into from

Conversation

ericrbg
Copy link
Collaborator

@ericrbg ericrbg commented Dec 13, 2021

Precursor to #10692.


Open in Gitpod

My brain is too frazzled right now, but here's a proof with one sorry of the same fact without the fintype requirement (for fintype, order_of_pos immediately gets us back to this version).

@ericrbg ericrbg added help-wanted The author needs attention to resolve issues awaiting-review The author would like community review of the PR labels Dec 13, 2021
@github-actions github-actions bot added the blocked-by-other-PR This PR depends on another PR which is still in the queue. A bot manages this label via PR comment. label Dec 13, 2021
@ericrbg ericrbg removed the help-wanted The author needs attention to resolve issues label Dec 13, 2021
@ericrbg
Copy link
Collaborator Author

ericrbg commented Dec 13, 2021

I finished that one sorry!

@github-actions github-actions bot added the merge-conflict Please `git merge origin/master` then a bot will remove this label. label Dec 14, 2021
@github-actions github-actions bot removed the blocked-by-other-PR This PR depends on another PR which is still in the queue. A bot manages this label via PR comment. label Dec 14, 2021
@github-actions
Copy link

🎉 Great news! Looks like all the dependencies have been resolved:

💡 To add or remove a dependency please update this issue/PR description.

Brought to you by Dependent Issues (:robot: ). Happy coding!

@github-actions github-actions bot removed the merge-conflict Please `git merge origin/master` then a bot will remove this label. label Dec 14, 2021
end

@[to_additive]
lemma exponent_eq_zero_of_order_zero {g : G} (hg : order_of g = 0) : exponent G = 0 :=
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should the name rather be exponent_eq_zero_of_order_of_eq_zero?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I feel like in the rest of the file, order has been used interchangably with order_of; I can change it but I prefer snappier names (at least as much as possible within the naming conventions)

Copy link
Member

Choose a reason for hiding this comment

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

Should we rename order_of to order? And update all the names? (Not in this PR, of course.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'd be in favour of that!

@github-actions github-actions bot added the merge-conflict Please `git merge origin/master` then a bot will remove this label. label Dec 15, 2021
@github-actions github-actions bot removed the merge-conflict Please `git merge origin/master` then a bot will remove this label. label Dec 17, 2021
ericrbg and others added 2 commits December 18, 2021 09:37
Co-authored-by: Johan Commelin <johan@commelin.net>

variable [cancel_comm_monoid G]

@[to_additive] lemma exponent_eq_Sup_order_of (h : ∀ g : G, 0 < order_of g) :
Copy link
Member

Choose a reason for hiding this comment

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

Do you see any way to split/golf this proof? Intuitively, I think it looks a lot longer than I would expect a proof of this result to be. But I haven't loaded it into VScode yet.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

my only thoughts are that the first half could maybe be made into a private lemma, and the second half could be turned into a calc; but I'm fairly bad at calc. If you want me to turn the first edge case into a separate lemma, though, I can do that!

Copy link
Collaborator Author

@ericrbg ericrbg Dec 18, 2021

Choose a reason for hiding this comment

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

the order arguments were a lot more finicky here than I expected; I wish I could just say "if g has order not dividing t (which maximises the group's order), then g*t clearly has larger order"; but this statement requires a fair bit more care than I thought

@jcommelin
Copy link
Member

I've extracted the first half of the big proof into a lemma. Because I think it is a generally useful statement.
The other half of the big proof still feels slow to me.

@ericrbg
Copy link
Collaborator Author

ericrbg commented Dec 22, 2021

thanks! it does look very useful, now that you extracted it. I think I sped up the proof a lot, the main culprit was ac_refl, but there was misc other things too. I hope we can get tactics that ignore associativity performantly in Lean4!

pow_add, pow_add, pow_one, ←mul_assoc, ←mul_assoc, nat.div_mul_cancel, mul_assoc,
lt_mul_iff_one_lt_right $ h t, ←pow_succ'],
exact one_lt_pow hp.1.one_lt a.succ_ne_zero,
exact hpk
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm really confused as to why, but if you put this hpk where it belongs in the monster rw (after nat.div_mul_cancel) then the rewrite fails completely. ¯\_(ツ)_/¯

variable [cancel_comm_monoid G]

@[to_additive] lemma exponent_eq_Sup_order_of (h : ∀ g : G, 0 < order_of g) :
exponent G = Sup (set.range (order_of : G → ℕ)) :=
Copy link
Member

Choose a reason for hiding this comment

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

We usually use supr for Sup (set.range _). It's definitionally the same but comes with nice notation and lemmas.

Copy link
Collaborator Author

@ericrbg ericrbg Dec 23, 2021

Choose a reason for hiding this comment

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

Ooh, I didn't know about this. Is there an alternative for finset as well, or should I just leave that as max'?

Copy link
Collaborator Author

@ericrbg ericrbg Dec 23, 2021

Choose a reason for hiding this comment

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

I just ended up just rw supr at the start; it seems to me that for proving things about it the Sup API is a bit better. But the csupr induction seemed cool; if I was rewriting the proof, I'd probably use that.

@ericrbg ericrbg changed the title feat(group_theory/exponent): exponent G = Sup (range order_of) feat(group_theory/exponent): exponent G = ⨆ g : G, order_of g Dec 23, 2021
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 🎉

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 Dec 23, 2021
bors bot pushed a commit that referenced this pull request Dec 23, 2021
Precursor to #10692.



Co-authored-by: Eric <37984851+ericrbg@users.noreply.github.com>
Co-authored-by: Johan Commelin <johan@commelin.net>
@bors
Copy link

bors bot commented Dec 23, 2021

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title feat(group_theory/exponent): exponent G = ⨆ g : G, order_of g [Merged by Bors] - feat(group_theory/exponent): exponent G = ⨆ g : G, order_of g Dec 23, 2021
@bors bors bot closed this Dec 23, 2021
@bors bors bot deleted the ericrbg/exponent_lemmas branch December 23, 2021 20:58
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