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

MatrixMorphism_abstract: Move endomorphism invariants to category #37731

Merged
merged 13 commits into from
Apr 27, 2024

Conversation

mkoeppe
Copy link
Contributor

@mkoeppe mkoeppe commented Apr 2, 2024

We move the methods charpoly, det, fcp, trace from MatrixMorphism_abstract to the Homsets.Endset.ElementMethods of the category FiniteDimensionalModulesWithBasis. Likewise, we move the method minpoly there from FreeModuleMorphism.

This makes them also available to endomorphisms of CombinatorialFreeModule.

We also deprecate the functions is_MatrixMorphism, is_FreeModuleMorphism, is_VectorSpaceMorphism (as part of #32414); they were almost unused.

📝 Checklist

  • The title is concise and informative.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.
  • I have created tests covering the changes.
  • I have updated the documentation accordingly.

⌛ Dependencies

@mkoeppe mkoeppe self-assigned this Apr 2, 2024
@mkoeppe mkoeppe changed the title MatrixMorphism_abstract: Move invariants to category MatrixMorphism_abstract: Move endomorphism invariants to category Apr 2, 2024
@mkoeppe mkoeppe requested review from tscrim and grhkm21 April 3, 2024 19:59
@tscrim
Copy link
Collaborator

tscrim commented Apr 4, 2024

There is an Endsets axiom for Homsets, so you can move those methods to that subcatregory.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Apr 4, 2024

https://github.com/sagemath/sage/blob/develop/src/sage/categories/modules.py#L818 seems to caution that Endset "is not used yet", but I'll try if this works.

@tscrim
Copy link
Collaborator

tscrim commented Apr 6, 2024

Two trivial failures in src/sage/categories/modules_with_basis.py.

return isinstance(x, FreeModuleMorphism)


class FreeModuleMorphism(matrix_morphism.MatrixMorphism):

minimal_polynomial = minpoly = FiniteDimensionalModulesWithBasis.Homsets.Endset.ElementMethods.minimal_polynomial
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this necessary? Shouldn't this come from the category?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've removed it

@mkoeppe mkoeppe force-pushed the MatrixMorphism_to_category branch from d5e3c54 to c2769ce Compare April 8, 2024 23:58
Copy link

github-actions bot commented Apr 9, 2024

Documentation preview for this PR (built with commit 84b0b23; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

@mkoeppe mkoeppe force-pushed the MatrixMorphism_to_category branch from c2769ce to 84b0b23 Compare April 18, 2024 04:25
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Apr 23, 2024

Any further concerns, or can we merge it?

Copy link
Collaborator

@tscrim tscrim left a comment

Choose a reason for hiding this comment

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

Yes, I think this can be merged.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Apr 23, 2024

Thanks!

@vbraun vbraun merged commit 9aca7a3 into sagemath:develop Apr 27, 2024
14 checks passed
@mkoeppe mkoeppe added this to the sage-10.4 milestone Apr 27, 2024
vbraun pushed a commit to vbraun/sage that referenced this pull request Apr 28, 2024
    
<!-- ^ 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
vbraun pushed a commit to vbraun/sage that referenced this pull request May 2, 2024
    
<!-- ^ 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
vbraun pushed a commit to vbraun/sage that referenced this pull request May 4, 2024
…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
vbraun pushed a commit to vbraun/sage that referenced this pull request May 9, 2024
…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
vbraun pushed a commit to vbraun/sage that referenced this pull request May 11, 2024
…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
vbraun pushed a commit to vbraun/sage that referenced this pull request May 12, 2024
…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
vbraun pushed a commit to vbraun/sage that referenced this pull request May 12, 2024
…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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants