sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.34k stars 452 forks source link

Matrix-wise functions for univariate polynomial matrices #31471

Closed vneiger closed 3 years ago

vneiger commented 3 years ago

Some polynomial-type operations should be made available matrix-wise:

To be decided whether this should be added:

CC: @ke456

Component: linear algebra

Keywords: polynomial matrix, univariate polynomials

Author: Vincent Neiger

Branch/Commit: 2c36a1e

Reviewer: Seung Gyu Hyun

Issue created by migration from https://trac.sagemath.org/ticket/31471

vneiger commented 3 years ago

Branch: u/gh-vneiger/matrix_wise_functions_for_univariate_polynomial_matrices

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Commit: 7c9e9a3

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

7c9e9a3matrix truncate
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

fd88fb1minor fixes
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from 7c9e9a3 to fd88fb1

vneiger commented 3 years ago
comment:4

Update:

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from fd88fb1 to ff70518

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

ff70518shift
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

34aa2efreverse, constant matrix
3b32934using constant_matrix where applicable
7990380is_constant; and some notes
85583a6matrix coefficient of degree, and some related updates
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from ff70518 to 85583a6

vneiger commented 3 years ago
comment:7

All the listed enhancements have been realized, along with tests and documentation. The methods support uniform actions, and also row-wise or column-wise ones where applicable (i.e. all methods except constant_matrix and is_constant).

Regarding naming:

d63dc5db-16cb-4358-9b00-8fd86f82a8e4 commented 3 years ago
comment:9

Things look good for the most part. Two suggestions: 1) for reverse, we should have an option to reverse polynomials wrt its own degree (rather than the degree of the polynomial matrix) when degrees are not specified - perhaps we can add a parameter entry_wise=False; 2) coefficient_matrix might be a less verbose name than matrix_coefficient_of_degree

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

d08c5b6fix required suggestions
5a95c6dsimplifying the code for matrices having one nrows or ncols zero
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from 85583a6 to 5a95c6d

vneiger commented 3 years ago
comment:11

Thank you for the two suggestions: I followed them for the additional entry_wise parameter, and for the coefficient_matrix name (less verbose, and also less confusing than things which would be too close to the coefficients and terms methods).

I also simplified the code a bit, since there was harmless but unnecessary lines supposed to handle matrices having zero rows or zero columns (or both).

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from 5a95c6d to 2c36a1e

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

a220bb8doc missing blank line
2c36a1eimprove documentation
vneiger commented 3 years ago
comment:13

Ready for second round of reviewing, thank you.

d63dc5db-16cb-4358-9b00-8fd86f82a8e4 commented 3 years ago
comment:14

The new changes seem to address the previous comment fully.

vbraun commented 3 years ago

Changed branch from u/gh-vneiger/matrix_wise_functions_for_univariate_polynomial_matrices to 2c36a1e