Skip to content
This repository was archived by the owner on Jul 24, 2024. It is now read-only.

[Merged by Bors] - chore(linear_algebra/dfinsupp): make lsum a linear_equiv#6185

Closed
eric-wieser wants to merge 5 commits intomasterfrom
eric-wieser/dfinsupp.lsum-equiv
Closed

[Merged by Bors] - chore(linear_algebra/dfinsupp): make lsum a linear_equiv#6185
eric-wieser wants to merge 5 commits intomasterfrom
eric-wieser/dfinsupp.lsum-equiv

Conversation

@eric-wieser
Copy link
Copy Markdown
Member

@eric-wieser eric-wieser commented Feb 11, 2021

Zulip thread with a summary of the problem which required the nasty semimodule_of_linear_map present here.


@eric-wieser eric-wieser added the awaiting-review The author would like community review of the PR label Feb 11, 2021
Comment thread src/linear_algebra/dfinsupp.lean Outdated
Co-authored-by: Scott Morrison <scott.morrison@gmail.com>
Comment thread src/linear_algebra/dfinsupp.lean Outdated
Comment thread src/linear_algebra/dfinsupp.lean Outdated
Comment on lines +94 to +99
instance semimodule_of_linear_map [semiring S] [semimodule S N] [smul_comm_class R S N] :
by haveI : add_comm_monoid (Π₀ i, M i →ₗ[R] N) :=
@dfinsupp.add_comm_monoid _ (λ i, M i →ₗ[R] N) _;
exactI semimodule S (Π₀ (i : ι), M i →ₗ[R] N) :=
let unused := S in
@dfinsupp.semimodule _ (λ i, M i →ₗ[R] N) _ _ _ _
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
instance semimodule_of_linear_map [semiring S] [semimodule S N] [smul_comm_class R S N] :
by haveI : add_comm_monoid (Π₀ i, M i →ₗ[R] N) :=
@dfinsupp.add_comm_monoid _ (λ i, M i →ₗ[R] N) _;
exactI semimodule S (Π₀ (i : ι), M i →ₗ[R] N) :=
let unused := S in
@dfinsupp.semimodule _ (λ i, M i →ₗ[R] N) _ _ _ _
instance semimodule_of_linear_map [semiring S] [semimodule S N] [smul_comm_class R S N] :
@semimodule S (Π₀ (i : ι), M i →ₗ[R] N) _ (@dfinsupp.add_comm_monoid _ (λ i, M i →ₗ[R] N) _) :=
@dfinsupp.semimodule _ (λ i, M i →ₗ[R] N) _ _ _ _

@kim-em
Copy link
Copy Markdown
Collaborator

kim-em commented Feb 14, 2021

Besides the suggestion and perhaps toning down the comment (it's not quite as excruciating now, hopefully!), looks good to me.

bors d+

@bors
Copy link
Copy Markdown

bors Bot commented Feb 14, 2021

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

@kim-em kim-em added delegated The PR author may merge after reviewing final suggestions. and removed awaiting-review The author would like community review of the PR labels Feb 15, 2021
@eric-wieser
Copy link
Copy Markdown
Member Author

Found a slightly tidier solution by adding a second manual instance. This refers to a library note added in #6183, so shouldn't be put on the queue until that library note name is confirmed.

@github-actions github-actions Bot added the blocked-by-other-PR This PR depends on another PR which is still in the queue. A bot manages this label via PR comment. label Feb 16, 2021
@eric-wieser
Copy link
Copy Markdown
Member Author

bors r+

@bors
Copy link
Copy Markdown

bors Bot commented Feb 16, 2021

👎 Rejected by label

@eric-wieser eric-wieser added ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.) and removed blocked-by-other-PR This PR depends on another PR which is still in the queue. A bot manages this label via PR comment. delegated The PR author may merge after reviewing final suggestions. labels Feb 16, 2021
@eric-wieser
Copy link
Copy Markdown
Member Author

bors r+

@github-actions
Copy link
Copy Markdown

🎉 Great news! Looks like all the dependencies have been resolved:

💡 To add or remove a dependency please update this issue/PR description.

Brought to you by Dependent Issues (:robot: ). Happy coding!

bors Bot pushed a commit that referenced this pull request Feb 16, 2021
@bors
Copy link
Copy Markdown

bors Bot commented Feb 16, 2021

Pull request successfully merged into master.

Build succeeded:

@bors bors Bot changed the title chore(linear_algebra/dfinsupp): make lsum a linear_equiv [Merged by Bors] - chore(linear_algebra/dfinsupp): make lsum a linear_equiv Feb 16, 2021
@bors bors Bot closed this Feb 16, 2021
@bors bors Bot deleted the eric-wieser/dfinsupp.lsum-equiv branch February 16, 2021 21:18
b-mehta pushed a commit that referenced this pull request Apr 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants