-
Notifications
You must be signed in to change notification settings - Fork 297
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(ring_theory/localization): localization of a basis, again #18261
Conversation
This PR adds `basis.localization_localization`, which maps `basis ι R S` to `basis ι Rm Sm`, where `Rm` and `Sm` are the localizations of `R` and `S` at `m`. This differs from the existing `basis.localization` by localizing in two places at once. See also: #18150
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my opinion we can delete basis.localization completely, but if you want to keep it (I didn't check if we really use it) can you at least add a comment referring to basis.localization_localization
? Thanks!
I can't find it in use but I think it might be useful (at least the linear independent part definitely was useful). Also it turns out you were the one who asked for |
It's definitely my fault, but in my mind I wanted this one and I didn't check carefully the statement 😅 The point is that it is very rare (impossible unless |
Good point, I've decided to delete |
Thanks! bors d+ |
✌️ Vierkantor can now approve this pull request. To approve and merge a pull request, simply reply with |
bors merge |
This PR replaces `basis.localization` with `basis.localization_localization`, which maps `basis ι R S` to `basis ι Rm Sm`, where `Rm` and `Sm` are the localizations of `R` and `S` at `m`. This differs from the existing `basis.localization` by localizing in two places at once, since localizing only the coefficients is probably not useful. See also: #18150 Co-authored-by: Vierkantor <vierkantor@vierkantor.com>
Pull request successfully merged into master. Build succeeded: |
This PR replaces
basis.localization
withbasis.localization_localization
, which mapsbasis ι R S
tobasis ι Rm Sm
, whereRm
andSm
are the localizations ofR
andS
atm
. This differs from the existingbasis.localization
by localizing in two places at once, since localizing only the coefficients is probably not useful.See also: #18150