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: trivial morphisms in StructuredArrow #10244

Closed
wants to merge 3 commits into from

Conversation

TwoFX
Copy link
Member

@TwoFX TwoFX commented Feb 4, 2024


Open in Gitpod

@TwoFX TwoFX added awaiting-review The author would like community review of the PR t-category-theory Category theory labels Feb 4, 2024
/-- Variant of `homMk'` where both objects are applications of `mk`. -/
@[simps]
def mkPostcomp (f : S ⟶ T.obj Y) (g : Y ⟶ Y') : mk f ⟶ mk (f ≫ T.map g) where
left := eqToHom (by ext)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
left := eqToHom (by ext)
left := 𝟙 _

(There are other definitions in this file where eqToHom can be replaced by the identity.)
Otherwise, I am wondering whether we could get rid of (even non simp) lemmas which introduce eqToHom in the RHS. Do you think we really need them? (They could be removed in a separate PR.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for pointing out the unnecessary eqToHom.

Regarding your question, I'm making essential use of these lemmas #10245 (I even make them into local simp lemmas there). I think it's definitely valid to ask in which way these constructions are functorial, and stating the answer happens to require eqToHom. StructuredArrow is just one case where DTT means that eqToHom is unavoidable, given that it is usually fine to consider equality of morphisms, but not when they are objects in StructuredArrow, so I think that it makes sense to provide these tools to those people who are unfortunate enough to need them. What do you think?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Actually, it is possible not to use eqToHom at all in #10245 (see the very draft commit 0d51b85 in the branch comma_presheaf_jriou). I have just tried to make the file compile without eqToHom. Most of proofs which are changed follow a similar pattern, so that it may be possible to get better automation. Then, I am not completely sure that keeping eqToHoms here is necessary the best design.

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 really appreciate you taking the time to investigate this in detail. I am very happy to see that it is possible to get rid of eqToHom entirely. I had unsucessfully tried a similar approach to yours in an earlier (and much messier) version of the proof, so it's great to see that it's possible.

However, I'm not convinced that a construction of the form Quiver.Hom.op (CostructuredArrow.homMk (by exact 𝟙 _)) is a clear improvement over eqToHom _.

I see several disadvantages with the homMk (by exact 𝟙 _) approach:

  • It's classic defeq abuse to forcefully supply 𝟙 _ in a place that is not syntactically (only definitionally) equal to X ⟶ X. This is exactly the kind of thing that (in my experience) will require things like simp, dsimp, simp and erw down the line, with simp not applying lemmas (even simple ones like Category.comp_id) because some object in an implicit argument isn't quite right, etc.
  • Given that the functors we're dealing with in this proof are completely general presheaves (CostructuredArrow yoneda A)ᵒᵖ ⥤ Type v, it's quite difficult to actually make use of the nice definitional property (homMk (by exact 𝟙 _)).left = 𝟙 _ that this approach gets you. Indeed, you still need to group all the instances of F.map (CostructuredArrow.homMk (by exact 𝟙 _)).op together and then combine them, just like in the eqToHom approach.

The main advantage that I believe the eqToHom approach has is that I'd much rather deal with a problem I can see (eqToHom) than one I cannot see (some implicit argument being syntactically incorrect, forcing me to use some arcane combination of dsimp, simp only and erw). eqToHom is "honest" in the way that if I put eqToHom _ in some place, then its source and target will syntactically match what's expected there, and the simplifier automatically collecting and eliminating eqToHoms in the goal has worked really well for me in this proof.

Sorry for the long-winded message, but I have fought with various incarnations of this proof for a long time (in fact, apparently the first time I committed something related to this proof was in December 2022, so more than a year ago), and if I were to start over again, I would definitely want to go with the eqToHom version again.

@joelriou
Copy link
Collaborator

joelriou commented Feb 5, 2024

bors merge

@github-actions github-actions bot added ready-to-merge This PR has been sent to bors. and removed awaiting-review The author would like community review of the PR labels Feb 5, 2024
mathlib-bors bot pushed a commit that referenced this pull request Feb 5, 2024
@mathlib-bors
Copy link

mathlib-bors bot commented Feb 5, 2024

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title feat: trivial morphisms in StructuredArrow [Merged by Bors] - feat: trivial morphisms in StructuredArrow Feb 5, 2024
@mathlib-bors mathlib-bors bot closed this Feb 5, 2024
@mathlib-bors mathlib-bors bot deleted the structuredarrow-precomp branch February 5, 2024 10:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge This PR has been sent to bors. t-category-theory Category theory
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants