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] - refactor(group_theory/free_*): remove API duplicated by lift, promote lift functions to equivs #6311

Closed
wants to merge 6 commits into from

Conversation

eric-wieser
Copy link
Member

@eric-wieser eric-wieser commented Feb 19, 2021

This removes:

  • free_group.to_group.to_fun and free_group.to_group, as these are both subsumed by the stronger lift.
  • abelianization.hom_equiv as this is now abelianization.lift.symm
  • free_abelian_group.hom_equiv as this is now free_abelian_group.lift.symm

This removes `free_group.to_group.to_fun` and `free_group.to_group`, as these are both subsumed by the stronger `lift`.
@eric-wieser eric-wieser changed the title refactor(group_theory/free_group): remove duplicated API refactor(group_theory/free_*): remove API duplicated by lift, promote lift functions to equivs Feb 19, 2021
by the map `f`.

* `hom_equiv : (free_abelian_group α →+ A) ≃ (α → A)` : the bijection witnessing adjointness.
* `lift : (α → A) ≃ free_abelian_group α →+ A` : the group homomorphism induced
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 not sure whether this might be confusing to newcomers.

Copy link
Member Author

Choose a reason for hiding this comment

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

I suppose I could leave the docstring as it was, and just remove the entry about hom_equiv?

Copy link
Collaborator

@Julian-Kuelshammer Julian-Kuelshammer Feb 19, 2021

Choose a reason for hiding this comment

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

What about: the adjunction isomorphism between the free abelian group functor and the forgetful functor or the bijection witnessing adjointness between the free abelian group functor and the forgetful functor combined with something like: for a function `f: \alpha\to A` the unique group homomorphism `free_abelian_group \alpha\to+ A` can be accessed using `lift f` ? This is probably a bit much for experts, but for beginners it is maybe appropriate? Here it is maybe not as relevant, but maybe if one wants to be uniform over all such constructions, one could also explain how to access the restriction of g for a group homomorphism g.

Copy link
Member Author

Choose a reason for hiding this comment

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

I feel like the category theory interpretation is probably not something we want to include in the description - although it would be quite sensible to add cross-links to the actual definitions of the adjunctions (in a follow-up PR, from someone else who actually knows the lexicon to precisely describe these things!)

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think, there is a link to the category theory interpretation in the first paragraph of the module doc. Maybe that is sufficient?

@@ -19,7 +19,7 @@ functor from groups to types, see `algebra/category/Group/adjunctions`.
modulo the relation `a * x * x⁻¹ * b = a * b`.
* `mk`: the canonical quotient map `list (α × bool) → free_group α`.
* `of`: the canoical injection `α → free_group α`.
* `to_group f`: the canonical group homomorphism `free_group α →* G` given a group `G` and a
* `lift f`: the canonical group homomorphism `free_group α →* G` given a group `G` and a
Copy link
Collaborator

Choose a reason for hiding this comment

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

lift should have similar docstrings throughout.

Copy link
Member Author

Choose a reason for hiding this comment

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

Any suggestions for how best to document it?

@eric-wieser eric-wieser added the awaiting-review The author would like community review of the PR label Feb 19, 2021
@Julian-Kuelshammer
Copy link
Collaborator

LGTM. I don't have time at the moment to think about a suggestion for a universal docstring but can do so later. In principle I agree with the motivation of making things uniform and lift was easy to use for me as a relative beginner.

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 Feb 20, 2021
bors bot pushed a commit that referenced this pull request Feb 20, 2021
… lift functions to equivs (#6311)

This removes:

* `free_group.to_group.to_fun` and `free_group.to_group`, as these are both subsumed by the stronger `lift`.
* `abelianization.hom_equiv` as this is now `abelianization.lift.symm`
* `free_abelian_group.hom_equiv` as this is now `free_abelian_group.lift.symm`
@bors
Copy link

bors bot commented Feb 20, 2021

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title refactor(group_theory/free_*): remove API duplicated by lift, promote lift functions to equivs [Merged by Bors] - refactor(group_theory/free_*): remove API duplicated by lift, promote lift functions to equivs Feb 20, 2021
@bors bors bot closed this Feb 20, 2021
@bors bors bot deleted the eric-wieser/free_abelian_group-lift branch February 20, 2021 23:46
b-mehta pushed a commit that referenced this pull request Apr 2, 2021
… lift functions to equivs (#6311)

This removes:

* `free_group.to_group.to_fun` and `free_group.to_group`, as these are both subsumed by the stronger `lift`.
* `abelianization.hom_equiv` as this is now `abelianization.lift.symm`
* `free_abelian_group.hom_equiv` as this is now `free_abelian_group.lift.symm`
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