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: | sage-8.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 follow-up: ↓ 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 follow-up: ↓ 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 follow-up: ↓ 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