Opened 2 years ago
Closed 2 years ago
#23216 closed defect (fixed)
Matrix of a module morphism who domain is 0 is always 0x0
Reported by:  tscrim  Owned by:  

Priority:  major  Milestone:  sage8.0 
Component:  linear algebra  Keywords:  
Cc:  sagecombinat, nthiery, darij  Merged in:  
Authors:  Travis Scrimshaw  Reviewers:  Darij Grinberg 
Report Upstream:  N/A  Work issues:  
Branch:  6dbc521 (Commits)  Commit:  6dbc5212aedf04202973d9f069a48557b7f2d470 
Dependencies:  Stopgaps: 
Description (last modified by )
sage: X = CombinatorialFreeModule(QQ, []) sage: Y = CombinatorialFreeModule(QQ, [1,2,3]) sage: Hom(X,Y).zero().matrix() [] sage: Hom(X,Y).zero().matrix().parent() Full MatrixSpace of 0 by 0 dense matrices over Rational Field
The result should be a 3x0 matrix. We need to explicitly compute the dimension of the codomain and pass that to the matrix constructor.
Change History (9)
comment:1 Changed 2 years ago by
 Branch set to public/linear_algebra/correct_dim_module_morphism_matrix23216
 Commit set to 773a18cd3ed430bf179cedfac78fcd1630d24887
 Description modified (diff)
 Status changed from new to needs_review
comment:2 Changed 2 years ago by
 Commit changed from 773a18cd3ed430bf179cedfac78fcd1630d24887 to 0ccba7548c06208e443ff0120bd6825d0fe06234
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
0ccba75  Explicitly check the dimension of the codomain for the matrix constructor.

comment:3 Changed 2 years ago by
 Commit changed from 0ccba7548c06208e443ff0120bd6825d0fe06234 to 70af2c8f57a0c3ee50c7887ce06ffcd5c1dc64a7
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
70af2c8  Explicitly check the dimension of the codomain for the matrix constructor.

comment:4 Changed 2 years ago by
Good catch! But seeing that [the matrix
constructor](https://git.sagemath.org/sage.git/tree/src/sage/matrix/constructor.pyx) does a ton of input normalization but no coercingtothebasering, wouldn't it be better to sidestep it and just call MatrixSpace(base_ring, len(basis_keys), self.codomain().dimension(), sparse=False)([on_basis(x)._vector_() for x in basis_keys])
?
comment:5 Changed 2 years ago by
 Commit changed from 70af2c8f57a0c3ee50c7887ce06ffcd5c1dc64a7 to 6dbc5212aedf04202973d9f069a48557b7f2d470
Branch pushed to git repo; I updated commit sha1. New commits:
6dbc521  Use MatrixSpace instead of matrix constructor.

comment:6 Changed 2 years ago by
Good point, done (the default is for the matrix space to be dense).
comment:7 Changed 2 years ago by
LGTM! If the tests pass, set this to p_r.
comment:8 Changed 2 years ago by
 Reviewers set to Darij Grinberg
 Status changed from needs_review to positive_review
Thank you.
comment:9 Changed 2 years ago by
 Branch changed from public/linear_algebra/correct_dim_module_morphism_matrix23216 to 6dbc5212aedf04202973d9f069a48557b7f2d470
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
Explicitly check the dimension of the codomain for the matrix constructor.