Opened 3 years ago

Closed 3 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: sage-8.0
Component: linear algebra Keywords:
Cc: sage-combinat, 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 tscrim)

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 3 years ago by tscrim

  • Branch set to public/linear_algebra/correct_dim_module_morphism_matrix-23216
  • Commit set to 773a18cd3ed430bf179cedfac78fcd1630d24887
  • Description modified (diff)
  • Status changed from new to needs_review

New commits:

773a18cExplicitly check the dimension of the codomain for the matrix constructor.

comment:2 Changed 3 years ago by git

  • Commit changed from 773a18cd3ed430bf179cedfac78fcd1630d24887 to 0ccba7548c06208e443ff0120bd6825d0fe06234

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

0ccba75Explicitly check the dimension of the codomain for the matrix constructor.

comment:3 Changed 3 years ago by git

  • Commit changed from 0ccba7548c06208e443ff0120bd6825d0fe06234 to 70af2c8f57a0c3ee50c7887ce06ffcd5c1dc64a7

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

70af2c8Explicitly check the dimension of the codomain for the matrix constructor.

comment:4 Changed 3 years ago by darij

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 coercing-to-the-base-ring, 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 3 years ago by git

  • Commit changed from 70af2c8f57a0c3ee50c7887ce06ffcd5c1dc64a7 to 6dbc5212aedf04202973d9f069a48557b7f2d470

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

6dbc521Use MatrixSpace instead of matrix constructor.

comment:6 Changed 3 years ago by tscrim

Good point, done (the default is for the matrix space to be dense).

comment:7 Changed 3 years ago by darij

LGTM! If the tests pass, set this to p_r.

comment:8 Changed 3 years ago by tscrim

  • Reviewers set to Darij Grinberg
  • Status changed from needs_review to positive_review

Thank you.

comment:9 Changed 3 years ago by vbraun

  • Branch changed from public/linear_algebra/correct_dim_module_morphism_matrix-23216 to 6dbc5212aedf04202973d9f069a48557b7f2d470
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.