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

method submatrix of matrix_mod2_dense needs default arguments #18761

Closed
cnassau opened this issue Jun 22, 2015 · 10 comments
Closed

method submatrix of matrix_mod2_dense needs default arguments #18761

cnassau opened this issue Jun 22, 2015 · 10 comments

Comments

@cnassau
Copy link

cnassau commented Jun 22, 2015

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)

CC: @malb

Component: linear algebra

Keywords: dense matrix, sub matrix

Author: Christian Nassau

Branch/Commit: 3549edf

Reviewer: Martin Albrecht

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

@cnassau cnassau added this to the sage-6.8 milestone Jun 22, 2015
@cnassau
Copy link
Author

cnassau commented Jun 22, 2015

Commit: 3549edf

@cnassau
Copy link
Author

cnassau commented Jun 22, 2015

New commits:

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

@cnassau
Copy link
Author

cnassau commented Jun 22, 2015

Branch: u/cnassau/submatrix_signature

@cnassau
Copy link
Author

cnassau commented Jun 22, 2015

comment:2

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.

@cnassau
Copy link
Author

cnassau commented Jun 22, 2015

Author: Christian Nassau

@malb
Copy link
Member

malb commented Jun 23, 2015

comment:5

Looks good to me.

@vbraun
Copy link
Member

vbraun commented Jun 24, 2015

comment:6

Reviewer name is missing

@cnassau
Copy link
Author

cnassau commented Jun 24, 2015

comment:7

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

@cnassau
Copy link
Author

cnassau commented Jun 24, 2015

Reviewer: Martin Albrecht

@vbraun
Copy link
Member

vbraun commented Jun 24, 2015

Changed branch from u/cnassau/submatrix_signature to 3549edf

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants