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/perm/cycle_type): Cycle type of a permutation #6999

Closed
wants to merge 31 commits into from

Conversation

tb65536
Copy link
Collaborator

@tb65536 tb65536 commented Apr 2, 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 Apr 2, 2021
@github-actions github-actions bot removed the merge-conflict Please `git merge origin/master` then a bot will remove this label. label Apr 6, 2021
@tb65536 tb65536 added the awaiting-review The author would like community review of the PR label Apr 6, 2021
@bryangingechen
Copy link
Collaborator

(I canceled the builds of fc5b7c4 and 35c67a3 because our GitHub actions queue is backed up.)

@tb65536 tb65536 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 Apr 6, 2021
@github-actions
Copy link

github-actions bot commented Apr 6, 2021

@@ -820,4 +821,27 @@ gcd_monoid_of_lcm
(λ a b c ac ab, normalize_dvd_iff.2 ((classical.some_spec (h c b) a).1 ⟨ac, ab⟩))
(λ a b, normalize_idem _)

/-- `ℕ` is a `gcd_monoid` -/
instance : gcd_monoid ℕ :=
Copy link
Member

Choose a reason for hiding this comment

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

@awainverse was this not already somewhere else? You know this part of the library best. Is this the correct place for the instance, or should it go somewhere in data/nat/*?

Copy link
Collaborator

Choose a reason for hiding this comment

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

It's been a minute, but it looks like the only global instances of gcd_monoid anywhere are polynomial.gcd_monoid and int.gcd_monoid. There is also unique_factorization_monoid.to_gcd_monoid, which is not an instance because it would cause a loop. Perhaps it's better if this just goes in ring_theory/int/basic next to int.gcd_monoid? I told @tb65536 it could go in either place.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It seems weird to me to put this in ring_theory/int/basic when this doesn't use facts about the integers.

Copy link
Collaborator

Choose a reason for hiding this comment

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

(In case it's not clear, either place is actually still fine with me)

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 should mention that the gcd_monoid instance is also in #7180

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm happy to close #7180 but I'm wondering whether it makes more sense to deduce the normalization_monoid part from unique (units nat) which already exists - althought in the very weird ring_theory/int/basic. I think it would be better to move it to data/nat/basic where one can also find this fact (not as an instance).

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.

This is a very nice development. The statements look great. The proofs overall look pretty clean as well, I have some minor comments.
I haven't looked before at permutations in mathlib, but the library seems to work very nicely!

How far away are you to showing that two permutations are conjugates iff they have the same cycle type? That seems like a nice fact.

src/group_theory/perm/cycle_type.lean Outdated Show resolved Hide resolved
src/group_theory/perm/cycle_type.lean Outdated Show resolved Hide resolved
src/group_theory/perm/cycle_type.lean Outdated Show resolved Hide resolved
src/group_theory/perm/cycle_type.lean Outdated Show resolved Hide resolved
src/group_theory/perm/cycle_type.lean Outdated Show resolved Hide resolved
@awainverse
Copy link
Collaborator

@fpvandoorn, I have progress towards that conjugacy result at #7024, and most of the rest at the branch disjoint_conj. It's coming soon! We may also end up with a proof closer to what's familiar as @pechersky develops a function to construct a cycle from a list.

@fpvandoorn
Copy link
Member

bors d+

@bors
Copy link

bors bot commented Apr 14, 2021

✌️ tb65536 can now approve this pull request. To approve and merge a pull request, simply reply with bors r+. More detailed instructions are available here.

@tb65536
Copy link
Collaborator Author

tb65536 commented Apr 14, 2021

bors r+

@fpvandoorn
Copy link
Member

Generally please wait to bors r+ until Github actions has succeeded al steps, it's annoying if bors tries to merge a failing build (in this case it's probably fine, since the linter passed the previous commit).

@tb65536
Copy link
Collaborator Author

tb65536 commented Apr 14, 2021

Generally please wait to bors r+ until Github actions has succeeded al steps, it's annoying if bors tries to merge a failing build (in this case it's probably fine, since the linter passed the previous commit).

Ah, thanks for mentioning this.

@pechersky
Copy link
Collaborator

pechersky commented Apr 14, 2021

@fpvandoorn, I have progress towards that conjugacy result at #7024, and most of the rest at the branch disjoint_conj. It's coming soon! We may also end up with a proof closer to what's familiar as @pechersky develops a function to construct a cycle from a list.

You can view the https://github.com/leanprover-community/mathlib/tree/pechersky/list-rotate-cycle branch I have, which merged in some commits from your disjoint_conj branch. One can define a perm from a list via list.form_perm, with a proof that the support of that perm is equal to the list if the list is nodup. I've also shown that list.form_perm l is is_cycle when nodup l and l is at least two elements.

There is also a cycle type defined which is the quotient of lists by rotation. That also has a perm defined on it via a lift from list.

I haven't proved things about conjugacy, but I think with my refactor of perm.support, perm.disjoint, and the proofs I have about the way that form_perm works, like form_perm l (l.nth_le k _) = l.nth_le ((k + 1) % l.length) _, I hope that that makes it easier to prove things about such constructed perms.

One more note -- the support refactor is at #7118, generalizing perm.support to be set instead of finset. That breaks some of the proofs in the branch that are from disjoint_conj, and I haven't fixed that yet.

@tb65536 tb65536 added delegated The PR author may merge after reviewing final suggestions. and removed awaiting-review The author would like community review of the PR labels Apr 14, 2021
bors bot pushed a commit that referenced this pull request Apr 14, 2021
This PR defines the cycle type of a permutation.

At some point we should prove the bijection between partitions and conjugacy classes.



Co-authored-by: Aaron Anderson <65780815+awainverse@users.noreply.github.com>
Co-authored-by: Thomas Browning <tb65536@uw.edu>
@bors
Copy link

bors bot commented Apr 15, 2021

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title feat(group_theory/perm/cycle_type): Cycle type of a permutation [Merged by Bors] - feat(group_theory/perm/cycle_type): Cycle type of a permutation Apr 15, 2021
@bors bors bot closed this Apr 15, 2021
@bors bors bot deleted the cycle_type branch April 15, 2021 03:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
delegated The PR author may merge after reviewing final suggestions.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants