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(Algebra/Module): Use coercion from SemilinearEquivClass to SemilinearEquiv #11966

Closed
wants to merge 1 commit into from

Conversation

101damnations
Copy link
Collaborator

@101damnations 101damnations commented Apr 6, 2024


Add the coercion from types with a SemilinearEquivClass instance to linear equivs, a la #10758.

Open in Gitpod


/-- Reinterpret an element of a type of semilinear equivalences as a semilinear equivalence. -/
instance instCoeToSemilinearEquiv [RingHomInvPair σ σ'] [RingHomInvPair σ' σ]
[EquivLike F M M₂] [SemilinearEquivClass F σ M M₂] : CoeHead F (M ≃ₛₗ[σ] M₂) where
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
[EquivLike F M M₂] [SemilinearEquivClass F σ M M₂] : CoeHead F (M ≃ₛₗ[σ] M₂) where
[EquivLike F M M₂] [SemilinearEquivClass F σ M M₂] : CoeTC F (M ≃ₛₗ[σ] M₂) where

For Equiv and MulEquiv this was done using CoeTC, so I imagine we want that everywhere?

bors d+

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 really understand this stuff but judging by this comment it looks like CoeHead is preferred here

@mathlib-bors
Copy link

mathlib-bors bot commented Apr 7, 2024

✌️ 101damnations can now approve this pull request. To approve and merge a pull request, simply reply with bors r+. More detailed instructions are available here.

@101damnations
Copy link
Collaborator Author

101damnations commented Apr 7, 2024

So there's already a coercion from linear equivs to linear maps given by the projection LinearEquiv.toLinearMap, and the following fails if I put it where it makes sense (e.g. after line 202)

example (f : M ≃ₛₗ[σ] M₂) :
    f.toLinearMap = SemilinearMapClass.semilinearMap f := by
  with_reducible rfl

I don't really understand with_reducible, but I guess maybe I should redefine

instance : Coe (M ≃ₛₗ[σ] M₂) (M →ₛₗ[σ] M₂) :=
  ⟨toLinearMap⟩

on line 153 using SemilinearMapClass.semilinearMap, and deal with the fall out? I guess this potentially means removing a lot of toLinearMaps (and frequently having to replace them with type ascriptions....)

@101damnations 101damnations added the awaiting-review The author would like community review of the PR label Apr 7, 2024
@fpvandoorn
Copy link
Member

Thanks for the link. In that case, we should try to fix the existing coercions.

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 Apr 7, 2024
mathlib-bors bot pushed a commit that referenced this pull request Apr 7, 2024
@mathlib-bors
Copy link

mathlib-bors bot commented Apr 7, 2024

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title feat(Algebra/Module): Use coercion from SemilinearEquivClass to SemilinearEquiv [Merged by Bors] - feat(Algebra/Module): Use coercion from SemilinearEquivClass to SemilinearEquiv Apr 7, 2024
@mathlib-bors mathlib-bors bot closed this Apr 7, 2024
@mathlib-bors mathlib-bors bot deleted the semilinearequivcoe branch April 7, 2024 18:52
Louddy pushed a commit that referenced this pull request Apr 15, 2024
atarnoam pushed a commit that referenced this pull request Apr 16, 2024
uniwuni pushed a commit that referenced this pull request Apr 19, 2024
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants