Opened 3 years ago

Closed 3 years ago

#23399 closed enhancement (fixed)

Some additions to matrix_mod_gfpn_dense

Reported by: SimonKing Owned by:
Priority: major Milestone: sage-8.2
Component: packages: optional Keywords:
Cc: tscrim Merged in:
Authors: Simon King Reviewers: Vincent Delecroix
Report Upstream: N/A Work issues:
Branch: f85c173 (Commits) Commit: f85c17357b9139a35b3e20af7e39f936397d7db8
Dependencies: #23400 Stopgaps:

Description (last modified by SimonKing)

Ticket #21437 is supposed to be split into smaller chunks, and this is one of them: Add methods to get and set horizontal slices and obtain a flat list of matrix entries. Make the _rowlist_ method cpdef, and document it.

Change History (24)

comment:1 Changed 3 years ago by SimonKing

  • Branch set to u/SimonKing/some_additions_to_matrix_mod_gfpn_dense

comment:2 Changed 3 years ago by SimonKing

  • Authors set to Simon King
  • Commit set to 6f7caf9ced1e3afdcf316b557b5ed9dc3388c464
  • Description modified (diff)
  • Status changed from new to needs_review

New commits:

b1aef56Add MeatAxe patch that ensures correct value of FfCurrentRowSizeIo
a44a58aFix reading mtx-matrix from non-existent file
91ac93fMake pickling machine independent
225afe1Properly fill the last few columns of random meataxe matrices
f6722beAdd c(p)def functions to get/set horizontal matrix slices
6f7caf9Document and test _rowlist_()

comment:3 Changed 3 years ago by SimonKing

  • Dependencies set to #23352

comment:4 Changed 3 years ago by git

  • Commit changed from 6f7caf9ced1e3afdcf316b557b5ed9dc3388c464 to 35b4ee7648f0c100ba0766ecc124713edc883ca7

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

bd1f3f1Fix ticket number in deprecation warning
35b4ee7Merge branch 't/23352/meataxe_fixes' into t/23399/some_additions_to_matrix_mod_gfpn_dense

comment:5 Changed 3 years ago by SimonKing

It turns out that auto-merging of #23352 into #23400 doesn't work. I hate git. Therefore, I do a manual merge here and rebase #23400 on top of it.


New commits:

bd1f3f1Fix ticket number in deprecation warning
35b4ee7Merge branch 't/23352/meataxe_fixes' into t/23399/some_additions_to_matrix_mod_gfpn_dense

New commits:

bd1f3f1Fix ticket number in deprecation warning
35b4ee7Merge branch 't/23352/meataxe_fixes' into t/23399/some_additions_to_matrix_mod_gfpn_dense

comment:6 Changed 3 years ago by SimonKing

  • Status changed from needs_review to needs_work
  • Work issues set to refactor a couple of tickets

comment:7 Changed 3 years ago by SimonKing

  • Dependencies changed from #23352 to #23400

comment:8 Changed 3 years ago by SimonKing

Note to self: Work on this ticket. After all, the dependencies are now fixed.

comment:9 Changed 3 years ago by tscrim

  • Cc tscrim added
  • Milestone changed from sage-8.0 to sage-8.2

comment:10 Changed 3 years ago by SimonKing

The branch doesn't cleanly merge. I thus suggest to rebase the changes on top of the current development branch, that should contain all dependencies of this ticket.

comment:11 Changed 3 years ago by git

  • Commit changed from 35b4ee7648f0c100ba0766ecc124713edc883ca7 to 64539382184cb1770f1036a2d79cb8109aa0e981

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

comment:12 Changed 3 years ago by git

  • Commit changed from 64539382184cb1770f1036a2d79cb8109aa0e981 to 7aa404ec855e573ce205f42da41a8d912b7dc989

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

7aa404eMethods to set/get horizontal slices of MeatAxe matrices; document _rowlist_

comment:13 Changed 3 years ago by SimonKing

  • Description modified (diff)
  • Status changed from needs_work to needs_review

Thanks to breaking all MeatAxe-related changes into smaller chunks, the new commit here is relatively small: Just add two new methods and add documentation to an existing method.

Needs review!

comment:14 Changed 3 years ago by SimonKing

  • Work issues refactor a couple of tickets deleted

comment:15 follow-up: Changed 3 years ago by vdelecroix

Looks ok, but

  • please add spaces 0<=i<j<=self.Data.Nor -> 0 <= i < j <= self.Data.Nor.
  • first line of documentation string should end with a point.

comment:16 Changed 3 years ago by git

  • Commit changed from 7aa404ec855e573ce205f42da41a8d912b7dc989 to 084ec32540877e58e1cf353373b01db584bb94f2

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

084ec32Spaces around comparison operators, '.' at the end of first doc line

comment:17 in reply to: ↑ 15 Changed 3 years ago by SimonKing

Replying to vdelecroix:

Looks ok, but

  • please add spaces 0<=i<j<=self.Data.Nor -> 0 <= i < j <= self.Data.Nor.
  • first line of documentation string should end with a point.

Done!

comment:18 Changed 3 years ago by vdelecroix

  • Reviewers set to Vincent Delecroix
  • Status changed from needs_review to positive_review

comment:19 Changed 3 years ago by vbraun

  • Status changed from positive_review to needs_work
sage -t --long --warn-long 66.1 src/sage/matrix/matrix_gfpn_dense.pyx
**********************************************************************
File "src/sage/matrix/matrix_gfpn_dense.pyx", line 873, in sage.matrix.matrix_gfpn_dense.Matrix_gfpn_dense._rowlist_
Failed example:
    M
Expected:
    [      4     4*x   x + 3 4*x + 2 3*x + 4]
    [  x + 2 3*x + 1       3       0       3]
    [    3*x 2*x + 4       1       0     2*x]
    [4*x + 4 2*x + 3     4*x       1 3*x + 1]
    [3*x + 3   x + 3   x + 2   x + 1 3*x + 2]
Got:
    [      2 3*x + 2   x + 4       2 3*x + 1]
    [3*x + 1 4*x + 1 4*x + 3     3*x       4]
    [      2       0 4*x + 2 2*x + 3       4]
    [2*x + 2   x + 4 2*x + 2 4*x + 1 3*x + 1]
    [    3*x     3*x     3*x 2*x + 4   x + 1]
**********************************************************************
File "src/sage/matrix/matrix_gfpn_dense.pyx", line 879, in sage.matrix.matrix_gfpn_dense.Matrix_gfpn_dense._rowlist_
Failed example:
    M._rowlist_(1)
Exception raised:
    Traceback (most recent call last):
      File "/mnt/disk/home/release/Sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 517, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/mnt/disk/home/release/Sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 920, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.matrix.matrix_gfpn_dense.Matrix_gfpn_dense._rowlist_[2]>", line 1, in <module>
        M._rowlist_(Integer(1))
      File "sage/structure/element.pyx", line 486, in sage.structure.element.Element.__getattr__ (build/cythonized/sage/structure/element.c:4443)
        return self.getattr_from_category(name)
      File "sage/structure/element.pyx", line 499, in sage.structure.element.Element.getattr_from_category (build/cythonized/sage/structure/element.c:4552)
        return getattr_from_other_class(self, cls, name)
      File "sage/cpython/getattr.pyx", line 254, in sage.cpython.getattr.getattr_from_other_class (build/cythonized/sage/cpython/getattr.c:1901)
        raise dummy_attribute_error
    AttributeError: 'sage.matrix.matrix_generic_dense.Matrix_generic_dense' object has no attribute '_rowlist_'
**********************************************************************
File "src/sage/matrix/matrix_gfpn_dense.pyx", line 881, in sage.matrix.matrix_gfpn_dense.Matrix_gfpn_dense._rowlist_
Failed example:
    [M[1,i]._int_repr() for i in range(5)]
Expected:
    ['7', '16', '3', '0', '3']
Got:
    ['16', '21', '23', '15', '4']
**********************************************************************
File "src/sage/matrix/matrix_gfpn_dense.pyx", line 883, in sage.matrix.matrix_gfpn_dense.Matrix_gfpn_dense._rowlist_
Failed example:
    M._rowlist_(2,4)
Exception raised:
    Traceback (most recent call last):
      File "/mnt/disk/home/release/Sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 517, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/mnt/disk/home/release/Sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 920, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.matrix.matrix_gfpn_dense.Matrix_gfpn_dense._rowlist_[4]>", line 1, in <module>
        M._rowlist_(Integer(2),Integer(4))
      File "sage/structure/element.pyx", line 486, in sage.structure.element.Element.__getattr__ (build/cythonized/sage/structure/element.c:4443)
        return self.getattr_from_category(name)
      File "sage/structure/element.pyx", line 499, in sage.structure.element.Element.getattr_from_category (build/cythonized/sage/structure/element.c:4552)
        return getattr_from_other_class(self, cls, name)
      File "sage/cpython/getattr.pyx", line 254, in sage.cpython.getattr.getattr_from_other_class (build/cythonized/sage/cpython/getattr.c:1901)
        raise dummy_attribute_error
    AttributeError: 'sage.matrix.matrix_generic_dense.Matrix_generic_dense' object has no attribute '_rowlist_'
**********************************************************************
File "src/sage/matrix/matrix_gfpn_dense.pyx", line 885, in sage.matrix.matrix_gfpn_dense.Matrix_gfpn_dense._rowlist_
Failed example:
    [[M[i,j]._int_repr() for j in range(5)] for i in range(2,5)]
Expected:
    [['15', '14', '1', '0', '10'],
     ['24', '13', '20', '1', '16'],
     ['18', '8', '7', '6', '17']]
Got:
    [['2', '0', '22', '13', '4'],
     ['12', '9', '12', '21', '16'],
     ['15', '15', '15', '14', '6']]
**********************************************************************
1 item had failures:
   5 of   7 in sage.matrix.matrix_gfpn_dense.Matrix_gfpn_dense._rowlist_
    [173 tests, 5 failures, 29.52 s]
----------------------------------------------------------------------
sage -t --long --warn-long 66.1 src/sage/matrix/matrix_gfpn_dense.pyx  # 5 doctests failed
----------------------------------------------------------------------
Total time for all tests: 29.6 seconds
    cpu time: 28.9 seconds
    cumulative wall time: 29.5 seconds

comment:20 Changed 3 years ago by SimonKing

Apparently I forgot to mark a couple of tests as "optional". As you can see in the error message, the meataxe package isn't install on whatever machine the tests were done.

comment:21 Changed 3 years ago by git

  • Commit changed from 084ec32540877e58e1cf353373b01db584bb94f2 to f85c17357b9139a35b3e20af7e39f936397d7db8

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

f85c173Mark some meataxe tests as optional

comment:22 Changed 3 years ago by SimonKing

  • Status changed from needs_work to needs_review

Should be fixed.

comment:23 Changed 3 years ago by vdelecroix

  • Status changed from needs_review to positive_review

Looks like it is.

comment:24 Changed 3 years ago by vbraun

  • Branch changed from u/SimonKing/some_additions_to_matrix_mod_gfpn_dense to f85c17357b9139a35b3e20af7e39f936397d7db8
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.