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/subgroup/basic): Define characteristic subgroups #9921

Closed
wants to merge 6 commits into from

Conversation

tb65536
Copy link
Collaborator

@tb65536 tb65536 commented Oct 23, 2021

This PR defines characteristic subgroups and builds the basic API.

Getting @[to_additive] to work correctly was a bit tricky, so I mostly just copied the setup for subgroup.normal.


Open in Gitpod

@tb65536 tb65536 added the awaiting-review The author would like community review of the PR label Oct 23, 2021
Copy link
Member

@urkud urkud left a comment

Choose a reason for hiding this comment

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

Generally LGTM but I would wait for comments from one of our "algebra" maintainers.

attribute [class] characteristic

@[priority 100] instance normal_of_characteristic [h : H.characteristic] : H.normal :=
⟨λ a ha b, (set_like.ext_iff.mp (h.fixed (mul_aut.conj b)) a).mpr ha⟩
Copy link
Member

Choose a reason for hiding this comment

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

Can to_additive generate this instance?

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 can't seem to get it to work :(

Comment on lines +1054 to +1057
structure characteristic : Prop :=
(fixed : ∀ ϕ : G ≃* G, H.comap ϕ.to_monoid_hom = H)

attribute [class] characteristic
Copy link
Member

Choose a reason for hiding this comment

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

You can use

Suggested change
structure characteristic : Prop :=
(fixed : ∀ ϕ : G ≃* G, H.comap ϕ.to_monoid_hom = H)
attribute [class] characteristic
@[class] structure characteristic : Prop :=
(fixed : ∀ ϕ : G ≃* G, H.comap ϕ.to_monoid_hom = H)

to achieve the same goal.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There is a slight difference. With the current setup, if [h : characteristic H], then you can write h.fixed. With the proposed setup, you have to write characteristic.fixed. This is especially painful if h is not an instance (e.g., in characteristic_iff_comap_eq).

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 presume this is why subgroup.normal does the same thing)

⟨characteristic.fixed, characteristic.mk⟩

@[to_additive] lemma characteristic_iff_comap_le :
H.characteristic ↔ ∀ ϕ : G ≃* G, H.comap ϕ.to_monoid_hom ≤ H :=
Copy link
Member

Choose a reason for hiding this comment

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

Please mention this and the next lemmas in the docstring.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done (assuming you mean the docstrings for the definitions)

src/group_theory/subgroup/basic.lean Outdated Show resolved Hide resolved
src/group_theory/subgroup/basic.lean Outdated Show resolved Hide resolved
src/group_theory/subgroup/basic.lean Outdated Show resolved Hide resolved
src/data/equiv/mul_add_aut.lean Outdated Show resolved Hide resolved
@urkud urkud 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 Oct 24, 2021
@tb65536 tb65536 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 Oct 24, 2021
@jcommelin jcommelin requested a review from urkud October 25, 2021 18:06
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 Oct 26, 2021
bors bot pushed a commit that referenced this pull request Oct 26, 2021
…9921)

This PR defines characteristic subgroups and builds the basic API.

Getting `@[to_additive]` to work correctly was a bit tricky, so I mostly just copied the setup for `subgroup.normal`.



Co-authored-by: tb65536 <tb65536@users.noreply.github.com>
@bors
Copy link

bors bot commented Oct 26, 2021

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title feat(group_theory/subgroup/basic): Define characteristic subgroups [Merged by Bors] - feat(group_theory/subgroup/basic): Define characteristic subgroups Oct 26, 2021
@bors bors bot closed this Oct 26, 2021
@bors bors bot deleted the char_subgroup branch October 26, 2021 20:05
ericrbg pushed a commit that referenced this pull request Nov 9, 2021
…9921)

This PR defines characteristic subgroups and builds the basic API.

Getting `@[to_additive]` to work correctly was a bit tricky, so I mostly just copied the setup for `subgroup.normal`.



Co-authored-by: tb65536 <tb65536@users.noreply.github.com>
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

3 participants