Opened 3 years ago
Closed 3 years ago
#24445 closed defect (fixed)
Allow several implementations in random matrix
Reported by:  SimonKing  Owned by:  

Priority:  blocker  Milestone:  sage8.2 
Component:  linear algebra  Keywords:  
Cc:  jipilab, tscrim, vdelecroix  Merged in:  
Authors:  Simon King  Reviewers:  Jeroen Demeyer 
Report Upstream:  N/A  Work issues:  
Branch:  652ca07 (Commits, GitHub, GitLab)  Commit:  652ca074f02c743bd2693bb07909a866663b828e 
Dependencies:  Stopgaps: 
Description
With the optional meataxe package installed, I get
sage: K.<a>=FiniteField(3^2) sage: MS = MatrixSpace(K, 2, 5, implementation="generic") sage: type(MS.random_element()) <type 'sage.matrix.matrix_generic_dense.Matrix_generic_dense'>
which is good, but
sage: type(random_matrix(K, 2, 5, implementation="generic")) <type 'sage.matrix.matrix_gfpn_dense.Matrix_gfpn_dense'>
which is bad.
So, the random_matrix
constructor should take into account what was contributed in #23706.
Change History (14)
comment:1 Changed 3 years ago by
 Branch set to u/SimonKing/allow_several_implementations_in_random_matrix
comment:2 Changed 3 years ago by
 Commit set to 571c12fa24badd695978e8ecad9152aa4a9d4847
comment:3 Changed 3 years ago by
 Status changed from new to needs_review
comment:4 followup: ↓ 5 Changed 3 years ago by
Could you change the following doctest (matrix/special.py
line 403)
The default implementation of :meth:`~sage.matrix.matrix2.randomize` relies on the ``random_element()`` method for the base ring. The ``density`` and ``sparse`` keywords behave as described above. Since we have a different randomisation when using the optional meataxe package, we have to make sure that we use the default implementation in this test:: sage: K.<a>=FiniteField(3^2) sage: from sage.matrix.matrix_generic_dense import Matrix_generic_dense sage: MS = MatrixSpace(K, 2, 5) sage: MS._MatrixSpace__matrix_class = Matrix_generic_dense sage: random_matrix(K, 2, 5) [ 1 a 1 2*a + 1 2] [ 2*a a + 2 0 2 1]
It could now be much simpler without any special hack.
comment:5 in reply to: ↑ 4 Changed 3 years ago by
Replying to vdelecroix:
Could you change the following doctest (
matrix/special.py
line 403)
The first commit does this:
@@ 407,10 +419,7 @@ def random_matrix(ring, nrows, ncols=None, algorithm='randomize', *args, **kwds) that we use the default implementation in this test:: sage: K.<a>=FiniteField(3^2)  sage: from sage.matrix.matrix_generic_dense import Matrix_generic_dense  sage: MS = MatrixSpace(K, 2, 5)  sage: MS._MatrixSpace__matrix_class = Matrix_generic_dense  sage: random_matrix(K, 2, 5) + sage: random_matrix(K, 2, 5, implementation='generic') [ 1 a 1 2*a + 1 2] [ 2*a a + 2 0 2 1]
So, I did change that test. Actually the whole purpose of this ticket was to fix this test (that would fail because of #23706 when meataxe is installed).
comment:6 Changed 3 years ago by
Then I think that I should close #24449 as duplicate as Jeroen suggested.
comment:7 Changed 3 years ago by
 Priority changed from major to blocker
comment:8 followup: ↓ 10 Changed 3 years ago by
 Status changed from needs_review to needs_work
Doctest failure in src/sage/misc/lazy_import.pyx
comment:9 Changed 3 years ago by
 Commit changed from 571c12fa24badd695978e8ecad9152aa4a9d4847 to 652ca074f02c743bd2693bb07909a866663b828e
Branch pushed to git repo; I updated commit sha1. New commits:
652ca07  Fix a doctest in lazy_import

comment:10 in reply to: ↑ 8 Changed 3 years ago by
 Status changed from needs_work to needs_review
comment:11 followup: ↓ 12 Changed 3 years ago by
It seems there should be doctests/examples showing the use of MeatAxe
package as a backend.
comment:12 in reply to: ↑ 11 Changed 3 years ago by
Replying to dimpase:
It seems there should be doctests/examples showing the use of
MeatAxe
package as a backend.
See basically all tests in sage.matrix.matrix_gfpn_dense
comment:13 Changed 3 years ago by
 Reviewers set to Jeroen Demeyer
 Status changed from needs_review to positive_review
comment:14 Changed 3 years ago by
 Branch changed from u/SimonKing/allow_several_implementations_in_random_matrix to 652ca074f02c743bd2693bb07909a866663b828e
 Resolution set to fixed
 Status changed from positive_review to closed
Branch pushed to git repo; I updated commit sha1. New commits:
Fix changed random_matrix output in tests