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
FreeModuleAutomorphism: Add more invariants #37826
FreeModuleAutomorphism: Add more invariants #37826
Conversation
Documentation preview for this PR (built with commit 7cfc187; changes) is ready! 🎉 |
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.
LGTM.
Thanks, Travis! In a follow-up PR, I'll do the same for endomorphisms. |
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.
Apart from the minor comments below, LGTM.
return self._some_matrix().fcp | ||
|
||
@lazy_attribute | ||
def minimal_polynomial(self): |
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.
Shouldn't var
appear in the argument list?
def minimal_polynomial(self): | |
def minimal_polynomial(self, var): |
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.
No, because this is the implementation of the lazy attribute. When it is invoked, it returns the method of the matrix.
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.
OK, I see. This is unfortunate regarding the documentation as shown in the reference manual (https://deploy-preview-37826--sagemath.netlify.app/html/en/reference/tensor_free_modules/sage/tensor/modules/free_module_automorphism#sage.tensor.modules.free_module_automorphism.FreeModuleAutomorphism.minimal_polynomial) : the method minimal_polynomial
appears as if it takes no argument.
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.
I agree that this is unfortunate...
return next(iter(self._matrices.values())) | ||
|
||
@lazy_attribute | ||
def characteristic_polynomial(self): |
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.
Shouldn't var
appear in the argument list?
def characteristic_polynomial(self): | |
def characteristic_polynomial(self, var): |
determinant = det | ||
|
||
@lazy_attribute | ||
def fcp(self): |
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.
Same question as for characteristic_polynomial
:
def fcp(self): | |
def fcp(self, var): |
Co-authored-by: Eric Gourgoulhon <eric.gourgoulhon@obspm.fr>
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.
Thanks for the changes.
return self._some_matrix().fcp | ||
|
||
@lazy_attribute | ||
def minimal_polynomial(self): |
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.
OK, I see. This is unfortunate regarding the documentation as shown in the reference manual (https://deploy-preview-37826--sagemath.netlify.app/html/en/reference/tensor_free_modules/sage/tensor/modules/free_module_automorphism#sage.tensor.modules.free_module_automorphism.FreeModuleAutomorphism.minimal_polynomial) : the method minimal_polynomial
appears as if it takes no argument.
Thanks! |
<!-- ^ Please provide a concise and informative title. --> <!-- ^ Don't put issue numbers in the title, do this in the PR description below. --> <!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method to calculate 1 + 2". --> <!-- v Describe your changes below in detail. --> <!-- v Why is this change required? What problem does it solve? --> <!-- v If this PR resolves an open issue, please link to it here. For example, "Fixes sagemath#12345". --> We add more methods that complement the existing methods `det` and `trace`, by delegating to the existing methods of Sage matrices. This is parallel to what is done for ModulesWithBasis in: - sagemath#37731 ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [x] The title is concise and informative. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [x] I have created tests covering the changes. - [ ] I have updated the documentation and checked the documentation preview. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - sagemath#12345: short description why this is a dependency --> <!-- - sagemath#34567: ... --> URL: sagemath#37826 Reported by: Matthias Köppe Reviewer(s): Eric Gourgoulhon, Matthias Köppe, Travis Scrimshaw
…et`, add element methods for invariants `det`, `charpoly`, `fcp`, etc. <!-- ^ Please provide a concise and informative title. --> <!-- ^ Don't put issue numbers in the title, do this in the PR description below. --> <!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method to calculate 1 + 2". --> <!-- v Describe your changes below in detail. --> <!-- v Why is this change required? What problem does it solve? --> <!-- v If this PR resolves an open issue, please link to it here. For example, "Fixes sagemath#12345". --> We create - a parent subclass `FreeModuleEndset` (of `FreeModuleHomset`) - an element subclass `FiniteRankFreeModuleEndomorphism` (of `FiniteRankFreeModuleMorphism`) for the purpose of providing specific methods of endomorphisms, namely the endomorphism invariants `det`, `trace`, `charpoly`, etc. This is parallel to what is done - for ModulesWithBasis in sagemath#37731 - in sagemath#37826 ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [x] The title is concise and informative. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation and checked the documentation preview. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - sagemath#12345: short description why this is a dependency --> <!-- - sagemath#34567: ... --> URL: sagemath#37831 Reported by: Matthias Köppe Reviewer(s): Eric Gourgoulhon, Travis Scrimshaw
…et`, add element methods for invariants `det`, `charpoly`, `fcp`, etc. <!-- ^ Please provide a concise and informative title. --> <!-- ^ Don't put issue numbers in the title, do this in the PR description below. --> <!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method to calculate 1 + 2". --> <!-- v Describe your changes below in detail. --> <!-- v Why is this change required? What problem does it solve? --> <!-- v If this PR resolves an open issue, please link to it here. For example, "Fixes sagemath#12345". --> We create - a parent subclass `FreeModuleEndset` (of `FreeModuleHomset`) - an element subclass `FiniteRankFreeModuleEndomorphism` (of `FiniteRankFreeModuleMorphism`) for the purpose of providing specific methods of endomorphisms, namely the endomorphism invariants `det`, `trace`, `charpoly`, etc. This is parallel to what is done - for ModulesWithBasis in sagemath#37731 - in sagemath#37826 ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [x] The title is concise and informative. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation and checked the documentation preview. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - sagemath#12345: short description why this is a dependency --> <!-- - sagemath#34567: ... --> URL: sagemath#37831 Reported by: Matthias Köppe Reviewer(s): Eric Gourgoulhon, Travis Scrimshaw
…et`, add element methods for invariants `det`, `charpoly`, `fcp`, etc. <!-- ^ Please provide a concise and informative title. --> <!-- ^ Don't put issue numbers in the title, do this in the PR description below. --> <!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method to calculate 1 + 2". --> <!-- v Describe your changes below in detail. --> <!-- v Why is this change required? What problem does it solve? --> <!-- v If this PR resolves an open issue, please link to it here. For example, "Fixes sagemath#12345". --> We create - a parent subclass `FreeModuleEndset` (of `FreeModuleHomset`) - an element subclass `FiniteRankFreeModuleEndomorphism` (of `FiniteRankFreeModuleMorphism`) for the purpose of providing specific methods of endomorphisms, namely the endomorphism invariants `det`, `trace`, `charpoly`, etc. This is parallel to what is done - for ModulesWithBasis in sagemath#37731 - in sagemath#37826 ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [x] The title is concise and informative. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation and checked the documentation preview. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - sagemath#12345: short description why this is a dependency --> <!-- - sagemath#34567: ... --> URL: sagemath#37831 Reported by: Matthias Köppe Reviewer(s): Eric Gourgoulhon, Travis Scrimshaw
…et`, add element methods for invariants `det`, `charpoly`, `fcp`, etc. <!-- ^ Please provide a concise and informative title. --> <!-- ^ Don't put issue numbers in the title, do this in the PR description below. --> <!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method to calculate 1 + 2". --> <!-- v Describe your changes below in detail. --> <!-- v Why is this change required? What problem does it solve? --> <!-- v If this PR resolves an open issue, please link to it here. For example, "Fixes sagemath#12345". --> We create - a parent subclass `FreeModuleEndset` (of `FreeModuleHomset`) - an element subclass `FiniteRankFreeModuleEndomorphism` (of `FiniteRankFreeModuleMorphism`) for the purpose of providing specific methods of endomorphisms, namely the endomorphism invariants `det`, `trace`, `charpoly`, etc. This is parallel to what is done - for ModulesWithBasis in sagemath#37731 - in sagemath#37826 ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [x] The title is concise and informative. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation and checked the documentation preview. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - sagemath#12345: short description why this is a dependency --> <!-- - sagemath#34567: ... --> URL: sagemath#37831 Reported by: Matthias Köppe Reviewer(s): Eric Gourgoulhon, Travis Scrimshaw
…et`, add element methods for invariants `det`, `charpoly`, `fcp`, etc. <!-- ^ Please provide a concise and informative title. --> <!-- ^ Don't put issue numbers in the title, do this in the PR description below. --> <!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method to calculate 1 + 2". --> <!-- v Describe your changes below in detail. --> <!-- v Why is this change required? What problem does it solve? --> <!-- v If this PR resolves an open issue, please link to it here. For example, "Fixes sagemath#12345". --> We create - a parent subclass `FreeModuleEndset` (of `FreeModuleHomset`) - an element subclass `FiniteRankFreeModuleEndomorphism` (of `FiniteRankFreeModuleMorphism`) for the purpose of providing specific methods of endomorphisms, namely the endomorphism invariants `det`, `trace`, `charpoly`, etc. This is parallel to what is done - for ModulesWithBasis in sagemath#37731 - in sagemath#37826 ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [x] The title is concise and informative. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation and checked the documentation preview. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - sagemath#12345: short description why this is a dependency --> <!-- - sagemath#34567: ... --> URL: sagemath#37831 Reported by: Matthias Köppe Reviewer(s): Eric Gourgoulhon, Travis Scrimshaw
We add more methods that complement the existing methods
det
andtrace
, by delegating to the existing methods of Sage matrices.This is parallel to what is done for ModulesWithBasis in:
MatrixMorphism_abstract
: Move endomorphism invariants to category #37731📝 Checklist
⌛ Dependencies