Opened 10 years ago

Closed 3 years ago

# Optional args in Matrix_Mod submatrix method

Reported by: Owned by: dscharles jason, was minor sage-duplicate/invalid/wontfix linear algebra matrix, finite field, submatrix David Sanchez Charles Martin Albrecht, Simon Brandhorst N/A add examples to doctests

When computing the smith form of a matrix (over a finite field) it promts an error

```sage: AA = matrix(Zmod(2), [[1,1],[1,0],[0,1]])
sage: AA.smith_form()
```

It promts

```Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "_sage_input_100.py", line 10, in <module>
exec compile(u'open("___code___.py","w").write("# -*- coding: utf-8 -*-\\n" + _support_.preparse_worksheet_cell(base64.b64decode("QUEuc21pdGhfZm9ybSgp"),globals())+"\\n"); execfile(os.path.abspath("___code___.py"))
File "", line 1, in <module>

File "/tmp/tmpXWhShB/___code___.py", line 2, in <module>
exec compile(u'AA.smith_form()
File "", line 1, in <module>

File "matrix2.pyx", line 10512, in sage.matrix.matrix2.Matrix.smith_form (sage/matrix/matrix2.c:49285)
File "matrix_mod2_dense.pyx", line 1666, in sage.matrix.matrix_mod2_dense.Matrix_mod2_dense.submatrix (sage/matrix/matrix_mod2_dense.c:9696)
TypeError: submatrix() takes exactly 4 positional arguments (2 given)
```

It seems that the "Matrix_mod" submatrix method doesn't have "ncols" and "nrows" as optional parameters (like the base Matrix class).

I used Sage 4.8.0 in an Ubuntu 12.04 (32 bits).

### comment:1 Changed 10 years ago by dscharles

I attached a patch. I hope it's well done (It's my first one...)

### comment:2 Changed 10 years ago by dscharles

• Description modified (diff)

### comment:3 Changed 10 years ago by malb

• Authors set to David Sanchez Charles
• Reviewers set to Martin Albrecht
• Status changed from new to needs_review

Patch looks good to me.

### comment:4 Changed 10 years ago by malb

• Status changed from needs_review to positive_review

### Changed 10 years ago by dscharles

It also required some output changes.

### comment:5 Changed 10 years ago by dscharles

I'm sorry I didn't realized before, but it also was necessary to change the output in order to work properly. (Should I open another ticket?)

Before the last patch:

```sage: A = matrix([[1,1],[2,2],[3,3]])
sage: B = matrix(Zmod(2), [[1,1],[2,2],[3,3]])
sage: A.submatrix(2,2)
[]
sage: A.submatrix(2,2).ncols()
0
sage: A.submatrix(2,2).nrows()
1
sage: B
[]
sage: B.submatrix(2,2).ncols()
0
sage: B.submatrix(2,2).nrows()
0
```

After the patch

```sage: B.submatrix(2,2)
[]
sage: B.submatrix(2,2).ncols()
0
sage: B.submatrix(2,2).nrows()
1
```

With that done, the smith_form method worked with some (easy) examples.

### comment:6 Changed 10 years ago by malb

• Status changed from positive_review to needs_work

Looks good, can you merge the two patches? Also, if you change stuff like that you should switch from positive review to needs review below.

Merged patch

### comment:7 Changed 10 years ago by dscharles

• Status changed from needs_work to needs_review

### comment:8 Changed 10 years ago by malb

Looks good, but could you also add those examples from this ticket to the doctests to test that your code produces the desired outcome. Sorry for only mentioning that now!

### comment:9 Changed 8 years ago by jdemeyer

• Milestone changed from sage-5.11 to sage-5.12

### comment:10 Changed 8 years ago by vbraun_spam

• Milestone changed from sage-6.1 to sage-6.2

### comment:11 Changed 8 years ago by rws

• Status changed from needs_review to needs_work
• Work issues set to add examples to doctests

### comment:12 Changed 8 years ago by vbraun_spam

• Milestone changed from sage-6.2 to sage-6.3

### comment:13 Changed 7 years ago by vbraun_spam

• Milestone changed from sage-6.3 to sage-6.4

### comment:14 Changed 4 years ago by chapoton

• Milestone changed from sage-6.4 to sage-duplicate/invalid/wontfix
• Status changed from needs_work to needs_review

Let us close this old ticket. Works now:

```sage: AA = matrix(Zmod(2), [[1,1],[1,0],[0,1]])
sage: AA.smith_form()
(
[1 0]  [1 0 0]
[0 1]  [1 1 0]  [1 1]
[0 0], [1 1 1], [0 1]
)
```

### comment:15 Changed 4 years ago by sbrandhorst

• Status changed from needs_review to positive_review

### comment:16 Changed 4 years ago by sbrandhorst

• Reviewers changed from Martin Albrecht to Martin Albrecht, Simon Brandhorst

### comment:17 Changed 3 years ago by embray

• Resolution set to worksforme
• Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.