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/submodule): add smul_of_tower_mem #6712

Closed
wants to merge 5 commits into from

Conversation

eric-wieser
Copy link
Member

@eric-wieser eric-wieser commented Mar 16, 2021

This adds the lemmas:

  • sub_mul_action.smul_of_tower_mem
  • submodule.smul_of_tower_mem

And uses them to construct the new scalar actions:

  • sub_mul_action.mul_action'
  • sub_mul_action.is_scalar_tower
  • submodule.semimodule'
  • submodule.is_scalar_tower

With associated lemmas

  • sub_mul_action.coe_smul_of_tower
  • submodule.coe_smul_of_tower

The unprimed instance continue to hold their old values, and exist to speed up typeclass search; the same pattern we use for tensor_product.semimodule vs tensor_product.semimodule.


Open in Gitpod

Split from #5569, which opened a can of elaborator-weakening worms that broke every call site of submodule.smul_mem. It's easier to just add a new lemma, smul_mem_of_tower, and leave combining the lemmas into a single lemma for another time. We already do things this way for linear_map anyway.

A natural follow-up here is to ensure that the Z-module structure from #5430 gets transferred too, but I'm going to punt that to another time.

Comment on lines 1883 to 1890
convert (L m).has_fderiv_at.comp_has_fderiv_within_at x this },
-- `convert (L m).has_fderiv_at.comp_has_fderiv_within_at x this` no longer works.
convert has_fderiv_at.comp_has_fderiv_within_at x _ this,
rotate 2,
convert linear_isometry_equiv.has_fderiv_at _,
rotate 1,
convert L m using 1,
refl,
refl, },
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 can't work out what's going on here - (L m).has_fderiv_at is now a type error, but it's very difficult for me to work out which bits of the types mismatch.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe you already fixed it? When I change it back to check the error, I don't get an error.

Copy link
Member Author

Choose a reason for hiding this comment

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

Are you sure you had the right cache loaded? I tested this in the last commit it was still broken.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I am relatively sure. Maybe it's a lean 3.27 / lean 3.28 issue?

% git status
On branch eric-wieser/smul_mem_of_tower
Your branch is up to date with 'mathlib/eric-wieser/smul_mem_of_tower'.
% git pull
Already up to date.
% leanproject get-cache
Looking for local mathlib oleans
Found local mathlib oleans
% lean --make src/analysis/calculus/times_cont_diff.lean 
% lean --version
Lean (version 3.27.0, commit de35266fe596, Release)

Copy link
Collaborator

Choose a reason for hiding this comment

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

I pushed commit 659086a to a new branch, let's see what CI thinks.

Copy link
Member Author

Choose a reason for hiding this comment

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

Seems to work? Perhaps I was running into another hash collision?

Copy link
Collaborator

Choose a reason for hiding this comment

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

The commit passed the build stage, so can you merge it back into your branch?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done. Thanks for double-checking this, I probably screwed up somehow when I did.

@eric-wieser eric-wieser added the awaiting-review The author would like community review of the PR label Mar 16, 2021
@eric-wieser eric-wieser requested a review from urkud March 16, 2021 17:22
@eric-wieser eric-wieser marked this pull request as ready for review March 16, 2021 17:22
src/algebra/module/submodule.lean Outdated Show resolved Hide resolved
src/group_theory/group_action/sub_mul_action.lean Outdated Show resolved Hide resolved
src/group_theory/group_action/sub_mul_action.lean Outdated Show resolved Hide resolved
@Vierkantor
Copy link
Collaborator

Thanks!

bors r+

@github-actions github-actions bot 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 awaiting-review The author would like community review of the PR labels Mar 17, 2021
bors bot pushed a commit that referenced this pull request Mar 17, 2021
This adds the lemmas:

* `sub_mul_action.smul_of_tower_mem`
* `submodule.smul_of_tower_mem`

And uses them to construct the new scalar actions:

* `sub_mul_action.mul_action'`
* `sub_mul_action.is_scalar_tower`
* `submodule.semimodule'`
* `submodule.is_scalar_tower`

With associated lemmas

* `sub_mul_action.coe_smul_of_tower`
* `submodule.coe_smul_of_tower`

The unprimed instance continue to hold their old values, and exist to speed up typeclass search; the same pattern we use for `tensor_product.semimodule` vs `tensor_product.semimodule`.



Co-authored-by: Vierkantor <vierkantor@vierkantor.com>
@bors
Copy link

bors bot commented Mar 17, 2021

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title feat(algebra/module/submodule): add smul_of_tower_mem [Merged by Bors] - feat(algebra/module/submodule): add smul_of_tower_mem Mar 17, 2021
@bors bors bot closed this Mar 17, 2021
@bors bors bot deleted the eric-wieser/smul_mem_of_tower branch March 17, 2021 22:48
b-mehta pushed a commit that referenced this pull request Apr 2, 2021
This adds the lemmas:

* `sub_mul_action.smul_of_tower_mem`
* `submodule.smul_of_tower_mem`

And uses them to construct the new scalar actions:

* `sub_mul_action.mul_action'`
* `sub_mul_action.is_scalar_tower`
* `submodule.semimodule'`
* `submodule.is_scalar_tower`

With associated lemmas

* `sub_mul_action.coe_smul_of_tower`
* `submodule.coe_smul_of_tower`

The unprimed instance continue to hold their old values, and exist to speed up typeclass search; the same pattern we use for `tensor_product.semimodule` vs `tensor_product.semimodule`.



Co-authored-by: Vierkantor <vierkantor@vierkantor.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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.

None yet

2 participants