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: port CategoryTheory.Quotient #2339

Closed
wants to merge 11 commits into from

Conversation

mpenciak
Copy link
Collaborator


Open in Gitpod

@mpenciak mpenciak added WIP Work in progress mathlib-port This is a port of a theory file from mathlib. labels Feb 17, 2023
@mo271
Copy link
Collaborator

mo271 commented Feb 24, 2023

currently there are errors in three parts:

  • instance category
  • functor_map_eq_iff
  • lift_unique

@bottine bottine added awaiting-review and removed WIP Work in progress labels Mar 2, 2023
@bottine
Copy link
Collaborator

bottine commented Mar 2, 2023

I checked the names and things seem in order now. Two changes that I'd like confirmation on:

  • There was a commented-out deriving … line with an instance later on and a porting note explaining that the deriving … line didn't work. I just removed that commented-out deriving line, since the porting note is sufficiently informative I believe.
  • In the following definition of HomRel, the isEquiv constructor was in UpperCamelCase, but I thought it matched items one and five of the porting wiki so I changed it to lowerCamel, and now I'm thinking it should maybe just be is_equiv? it's confusing.
/-- A `HomRel` is a congruence when it's an equivalence on every hom-set, and it can be composed
from left and right. -/
class Congruence : Prop where
  /-- `r` is an equivalence on every hom-set. -/
  isEquiv : ∀ {X Y}, IsEquiv _ (@r X Y)
  /-- Precomposition with an arrow respects `r`. -/
  compLeft : ∀ {X Y Z} (f : X ⟶ Y) {g g' : Y ⟶ Z}, r g g' → r (f ≫ g) (f ≫ g')
  /-- Postcomposition with an arrow respects `r`. -/
  compRight : ∀ {X Y Z} {f f' : X ⟶ Y} (g : Y ⟶ Z), r f f' → r (f ≫ g) (f' ≫ g)
#align category_theory.congruence CategoryTheory.Congruence
  • Ah, and something else: there seems to be a bit of a duplication in the code: originally, it was shown that the composite of "quotient, then the lift of a functor" is isomorphic to the original functor, but then I added code showing that the functors are actually equal. It would make sense to reuse the latter to prove the former, or even remove it. But I guess that's for after the port. Does it deserve a porting note?

@jcommelin
Copy link
Member

In general equalities of functors can be quite taxing on the kernel. And mathematically it isn't a very well-behaved notion anyways. That's why we often only state isomorphisms.

from left and right. -/
class Congruence : Prop where
/-- `r` is an equivalence on every hom-set. -/
isEquiv : ∀ {X Y}, IsEquiv _ (@r X Y)
Copy link
Member

Choose a reason for hiding this comment

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

isEquiv looks good to me.

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

@semorrison semorrison added ready-to-merge This PR has been sent to bors. and removed awaiting-review labels Mar 2, 2023
bors bot pushed a commit that referenced this pull request Mar 2, 2023
Co-authored-by: Moritz Firsching <firsching@google.com>
Co-authored-by: Rémi Bottinelli <remi.bottinelli@bluewin.ch>
@bors
Copy link

bors bot commented Mar 2, 2023

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title feat: port CategoryTheory.Quotient [Merged by Bors] - feat: port CategoryTheory.Quotient Mar 2, 2023
@bors bors bot closed this Mar 2, 2023
@bors bors bot deleted the port/CategoryTheory.Quotient branch March 2, 2023 08:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mathlib-port This is a port of a theory file from mathlib. ready-to-merge This PR has been sent to bors.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants