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: fix LinearMap.coe_mk #2336

Closed
wants to merge 4 commits into from

Conversation

Komyyy
Copy link
Collaborator

@Komyyy Komyyy commented Feb 17, 2023


This PR fix LinearMap.coe_mk.

@[simp]
theorem coe_mk {σ : R →+* S} (f : M →+ M₃) (h) :
    ((LinearMap.mk f h : M →ₛₗ[σ] M₃) : M → M₃) = f

to

@[simp]
theorem coe_mk {σ : R →+* S} (f : M → M₃) (h₁ h₂) :
    ((LinearMap.mk (AddHom.mk f h₁) h₂ : M →ₛₗ[σ] M₃) : M → M₃) = f

Open in Gitpod

@ChrisHughes24
Copy link
Member

This seems like it makes the lemma less general. I would suggest maybe changing the statement of LinearMap.coe_mk to the following (changing the coercion from M → M₃ to M →+ M₃.

theorem coe_mk {σ : R →+* S} (f : M →+ M₃) (h) :
    ((LinearMap.mk f h : M →ₛₗ[σ] M₃) : M →+ M₃) = f

You could also add AddHom.coe_mk if it's not there already and then the combination of the two lemmas should be able to prove you version of LinearMap.coe_mk.

What do you think @eric-wieser ?

@eric-wieser
Copy link
Member

We should have both the version with coe and with coe_fn; I'm a little surprised to hear that Lean3 only has the second of these. Maybe it has both and we forgot to port one of them

@Komyyy Komyyy added WIP Work in progress and removed awaiting-review labels Feb 17, 2023
@Komyyy
Copy link
Collaborator Author

Komyyy commented Feb 17, 2023

AddHom.coe_mk is already in the library with a simp attribute, but simp couldn't prove my first version of LinearMap.coe_mk.
I felt something is wrong, and finally found that →+ is actually not AddHom but AddMonoidHom. 🤯
So, the best way is this.

@[simp]
theorem coe_mk {σ : R →+* S} (f : AddHom M M₃) (h) :
    ((LinearMap.mk f h : M →ₛₗ[σ] M₃) : M → M₃) = f

I also add this.

@[simp]
theorem coe_mk' {σ : R →+* S} (f : AddHom M M₃) (h) :
    ((LinearMap.mk f h : M →ₛₗ[σ] M₃) : AddHom M M₃) = f

@Komyyy Komyyy added awaiting-review and removed WIP Work in progress labels Feb 17, 2023
Komyyy and others added 2 commits February 17, 2023 23:50
Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
@urkud
Copy link
Member

urkud commented Feb 18, 2023

Thanks! 🎉
bors merge

@jcommelin
Copy link
Member

bors areyouasleep?

bors merge

@github-actions github-actions bot added ready-to-merge This PR has been sent to bors. and removed awaiting-review labels Feb 18, 2023
bors bot pushed a commit that referenced this pull request Feb 18, 2023
Co-authored-by: Pol_tta <52843868+Komyyy@users.noreply.github.com>
@bors
Copy link

bors bot commented Feb 18, 2023

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title refactor: fix LinearMap.coe_mk [Merged by Bors] - refactor: fix LinearMap.coe_mk Feb 18, 2023
@bors bors bot closed this Feb 18, 2023
@bors bors bot deleted the pol_tta/LinearMap.coe_mk branch February 18, 2023 18:52
mo271 pushed a commit that referenced this pull request Feb 18, 2023
Co-authored-by: Pol_tta <52843868+Komyyy@users.noreply.github.com>
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