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] - chore(LinearAlgebra/TensorProduct): fix duplication #17912

Closed
wants to merge 2 commits into from

Conversation

yuma-mizuno
Copy link
Collaborator

This was unintentionally made in #16776 by myself.


Open in Gitpod

@yuma-mizuno yuma-mizuno added easy < 20s of review time. See the lifecycle page for guidelines. t-algebra Algebra (groups, rings, fields, etc) labels Oct 18, 2024
Copy link

github-actions bot commented Oct 18, 2024

PR summary aa40cbc5b6

Import changes for modified files

No significant changes to the import graph

Import changes for all files
Files Import difference

Declarations diff

No declarations were harmed in the making of this PR! 🐙

You can run this locally as follows
## summary with just the declaration names:
./scripts/declarations_diff.sh <optional_commit>

## more verbose report:
./scripts/declarations_diff.sh long <optional_commit>

The doc-module for script/declarations_diff.sh contains some details about this script.

@mo271
Copy link
Collaborator

mo271 commented Oct 18, 2024

Do we do deprecation alias for those?

@yuma-mizuno
Copy link
Collaborator Author

Do we do deprecation alias for those?

I have added.

Copy link
Member

@eric-wieser eric-wieser left a comment

Choose a reason for hiding this comment

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

bors d+

If you wanted, you could deprecate the *fold names in favor of subscripts (the reverse of what you've done here) if you'd find them easier to discover. Of course, by ext x y z works fine anyway, so no one should really need these lemmas...

@mathlib-bors
Copy link
Contributor

mathlib-bors bot commented Oct 18, 2024

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

@yuma-mizuno
Copy link
Collaborator Author

Of course, by ext x y z works fine anyway, so no one should really need these lemmas...

In my understanding, ext x y z only works in the local ext setting in this file. I'm not sure why the setting is like this.

@yuma-mizuno
Copy link
Collaborator Author

The subscript name was an unintentional product, but it might not be a bad name.

@yuma-mizuno
Copy link
Collaborator Author

bors r+

@eric-wieser
Copy link
Member

Of course, by ext x y z works fine anyway, so no one should really need these lemmas...

In my understanding, ext x y z only works in the local ext setting in this file.

Yes this is true,

I'm not sure why the setting is like this.

This was a decision made near the end of mathlib3, for performance. We could try revisiting it now that we have better lean 4 and benchmarking infrastructure.

@yuma-mizuno
Copy link
Collaborator Author

bors r+

mathlib-bors bot pushed a commit that referenced this pull request Oct 20, 2024
This was unintentionally made in [#16776](#16776) by myself.
@mathlib-bors
Copy link
Contributor

mathlib-bors bot commented Oct 20, 2024

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title chore(LinearAlgebra/TensorProduct): fix duplication [Merged by Bors] - chore(LinearAlgebra/TensorProduct): fix duplication Oct 20, 2024
@mathlib-bors mathlib-bors bot closed this Oct 20, 2024
@mathlib-bors mathlib-bors bot deleted the ymizuno-dedup branch October 20, 2024 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
delegated easy < 20s of review time. See the lifecycle page for guidelines. t-algebra Algebra (groups, rings, fields, etc)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants