Opened 4 years ago

Closed 4 years ago

#18761 closed defect (fixed)

method submatrix of matrix_mod2_dense needs default arguments

Reported by: cnassau Owned by:
Priority: major Milestone: sage-6.8
Component: linear algebra Keywords: dense matrix, sub matrix
Cc: malb Merged in:
Authors: Christian Nassau Reviewers: Martin Albrecht
Report Upstream: N/A Work issues:
Branch: 3549edf (Commits) Commit: 3549edfeec4bdcc3976963b021f19807afd9eaa6
Dependencies: Stopgaps:

Description

The submatrix methods for dense matrices mod 2e have a different signature than the other submatrix methods; the latter treat their last two arguments as optional. This leads to errors like this (from 6.8.beta5):

sage: d0=matrix(GF(2),[[1, 1], [1, 1], [1, 1], [1, 1]])
sage: d0._echelon_form_PID()---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
...
TypeError: submatrix() takes exactly 4 positional arguments (2 given)

or this

sage: d0=matrix(GF(2),[[1, 1], [1, 1], [1, 1], [1, 1]])
sage: d1=matrix(GF(2),[[1, 1, 0, 0],
 [1, 1, 0, 0],
 [1, 1, 1, 1],
 [1, 1, 1, 1],
 [0, 0, 1, 1],
 [0, 0, 1, 1]]
)
sage: C=ChainComplex(data=(d0,d1))
sage: C.homology(1,generators=True)
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
...
TypeError: submatrix() takes exactly 4 positional arguments (2 given)

Change History (8)

comment:1 Changed 4 years ago by cnassau

  • Branch set to u/cnassau/submatrix_signature
  • Commit set to 3549edfeec4bdcc3976963b021f19807afd9eaa6
  • Status changed from new to needs_review

New commits:

3549edfTicket #18761 - change the signature of the submatrix method for dense

comment:2 Changed 4 years ago by cnassau

I have given the submatrix methods the same signature as in file "matrix1.pyx". This includes a name change of the arguments, because in parts of Sage (eg, ChainComplex._homology_generators_snf) the method is called with named arguments.

I also made this change:

-        if self._ncols == 0 or self._nrows == 0:
+        if ncols == 0 or nrows == 0:

This fixed a segfault in my tests, and also seems to make more sense that the original: one returns an uninitialized matrix if *it* is empty, not if *self* is empty.

Caveat: I have no detailed understanding of m4rie, someone knowledgeable might want to have a look at my changes.

comment:3 Changed 4 years ago by cnassau

  • Authors set to Christian Nassau

comment:4 Changed 4 years ago by cnassau

  • Cc malb added

comment:5 Changed 4 years ago by malb

  • Status changed from needs_review to positive_review

Looks good to me.

comment:6 Changed 4 years ago by vbraun

  • Status changed from positive_review to needs_work

Reviewer name is missing

comment:7 Changed 4 years ago by cnassau

  • Reviewers set to Martin Albrecht
  • Status changed from needs_work to positive_review

I have added the name of the reviewer, and taken the liberty to revert the status to positive-review again myself.

comment:8 Changed 4 years ago by vbraun

  • Branch changed from u/cnassau/submatrix_signature to 3549edfeec4bdcc3976963b021f19807afd9eaa6
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.