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

get rid of isgraph_arrow #1884

Merged
merged 2 commits into from
Mar 8, 2024
Merged

Conversation

Alizter
Copy link
Collaborator

@Alizter Alizter commented Mar 7, 2024

This instance is far too general and should be restricted to Universe.v where it is actually used. A few places in the library were picking this up. This can be fixed by changeing a -> to a $-> so that the correct instance is inferred.

@@ -124,7 +124,7 @@ Defined.

(* A group homomorphism consists of a map between groups and a proof that the map preserves the group operation. *)
Record GroupHomomorphism (G H : Group) := Build_GroupHomomorphism' {
grp_homo_map : G -> H;
grp_homo_map : group_type G -> group_type H;
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 originally tried making this $-> but it messes with the coercions. I quickly tweaked it to be the underlying type instead, but it can be undone. I think it is better this way.

@@ -238,7 +238,7 @@ Global Instance is01cat_cring : Is01Cat CRing
:= Build_Is01Cat _ _ rng_homo_id (@rng_homo_compose).

Global Instance is2graph_cring : Is2Graph CRing
:= fun A B => isgraph_induced (@rng_homo_map A B).
:= fun A B => isgraph_induced (@rng_homo_map A B : _ -> (cring_type A $-> _)).
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 don't know why this worked but in Group.v I needed to introduce it everywhere for the induced parts.

@Alizter Alizter requested a review from jdchristensen March 7, 2024 16:53
Copy link
Collaborator

@jdchristensen jdchristensen left a comment

Choose a reason for hiding this comment

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

I think it's reasonable to get rid of this overly general instance. Just a few suggestions.

Comment on lines 81 to 83
- nrapply isequiv_compose.
1: apply isequiv_ap_equiv_fun.
exact (isequiv_Htpy_path (uncore A) (uncore B) f g).
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
- nrapply isequiv_compose.
1: apply isequiv_ap_equiv_fun.
exact (isequiv_Htpy_path (uncore A) (uncore B) f g).
- rapply isequiv_compose.
rapply isequiv_Htpy_path.

Copy link
Collaborator

Choose a reason for hiding this comment

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

BTW, this whole result could be generalized and put into WildCat/Equiv.v, with the assumption that ap cate_fun is an equivalence. Then it could be used both here and for hasmorext_core_ptype.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Doesn't look like this is even used anywhere, but I'll put a more general version next to the core stuff for completeness.

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'm gonna leave this for another PR as I think I can make a few more improvements.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sure, makes sense.

theories/Algebra/Groups/Group.v Outdated Show resolved Hide resolved
Alizter added 2 commits March 8, 2024 15:13
This instance is far too general and should be restricted to Universe.v
where it is actually used. A few places in the library were picking this
up. This can be fixed by changeing a -> to a $-> so that the correct
instance is inferred.

Signed-off-by: Ali Caglayan <[email protected]>

<!-- ps-id: feda50b3-daa5-4e3b-8205-59704ea2b1db -->
Signed-off-by: Ali Caglayan <[email protected]>

<!-- ps-id: a5fdb34c-094f-4e8d-bd37-85e1d9657cd1 -->
@Alizter Alizter force-pushed the ps/rr/get_rid_of_isgraph_arrow branch from 17675b6 to 47b62b7 Compare March 8, 2024 14:13
@Alizter
Copy link
Collaborator Author

Alizter commented Mar 8, 2024

I'll merge this when it is green.

@Alizter Alizter merged commit 019fd76 into HoTT:master Mar 8, 2024
21 checks passed
@Alizter Alizter deleted the ps/rr/get_rid_of_isgraph_arrow branch March 8, 2024 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants