Opened 6 years ago
Closed 6 years ago
#18177 closed defect (fixed)
Warning to Matrix.random()
Reported by:  jmantysalo  Owned by:  

Priority:  minor  Milestone:  sage6.6 
Component:  linear algebra  Keywords:  
Cc:  rbeezer  Merged in:  
Authors:  Jori Mäntysalo  Reviewers:  Nathann Cohen 
Report Upstream:  N/A  Work issues:  
Branch:  60da7a8 (Commits)  Commit:  60da7a81e17caa667f0429b5a149d8fcfa9af1d5 
Dependencies:  Stopgaps: 
Description
Matrix.random()
do not give uniform distribution. What's worse, for GF matrices it does not even generate them all. Until this is fixed at least warning is needed in documentation.
Change History (12)
comment:1 Changed 6 years ago by
 Branch set to u/jmantysalo/warning_to_matrix_random__
comment:2 Changed 6 years ago by
 Cc rbeezer added
 Commit set to 371e40fe64f300e48ceeacc69f9d09405674b3a3
 Status changed from new to needs_review
comment:3 followup: ↓ 4 Changed 6 years ago by
Looks good. A naive request: if there is an example of small order for which you are sure that a given matrix M
is never returned, could you add a doctest/example to this warning to illustrate it?
Something like all(a_random_matrix() != m for i in range(100))
?
Nathann
comment:4 in reply to: ↑ 3 ; followup: ↓ 5 Changed 6 years ago by
Replying to ncohen:
Looks good. A naive request: if there is an example of small order for which you are sure that a given matrix
M
is never returned, could you add a doctest/example to this warning to illustrate it?Something like
all(a_random_matrix() != m for i in range(100))
?
Hmm... For unimodular 3x3
matrices over GF(3)
there seems to be 3^5
different matrix that are never returned. But for 2x2
over GF(2)
there are only 3 of those. Easiest example is M=Matrix([[0,2],[2,0]])
. But is this information useful to user?
comment:5 in reply to: ↑ 4 ; followup: ↓ 6 Changed 6 years ago by
Hmm... For unimodular
3x3
matrices overGF(3)
there seems to be3^5
different matrix that are never returned. But for2x2
overGF(2)
there are only 3 of those. Easiest example isM=Matrix([[0,2],[2,0]])
. But is this information useful to user?
Well, it would just be a 'proof' of what we say. But M=Matrix([[0,2],[2,0]])
does not seem to be a very good example of a matrix over GF(2)
^^;
Nathann
comment:6 in reply to: ↑ 5 Changed 6 years ago by
Replying to ncohen:
Well, it would just be a 'proof' of what we say. But
M=Matrix([[0,2],[2,0]])
does not seem to be a very good example of a matrix overGF(2)
^^;
Sorry, I meant 2x2
over GF(3)
. But the question still remains: is this useful for user?
comment:7 followup: ↓ 8 Changed 6 years ago by
It answers some questions that one could ask, like: "Okayy, it says that it does not always work but perhaps it will work for me because I deal with very small matrices over very small fields?". And the answer is 'no'. It is not very important, I was just thinking that it would be cool to say "not all matrices appear, for example: <sage code>".
If you think that it is too much trouble, no problem.
comment:8 in reply to: ↑ 7 Changed 6 years ago by
 Status changed from needs_review to needs_work
Replying to ncohen:
It answers some questions that one could ask, like: "Okayy, it says that it does not always work but perhaps it will work for me because I deal with very small matrices over very small fields?". And the answer is 'no'. It is not very important, I was just thinking that it would be cool to say "not all matrices appear, for example: <sage code>".
OK, I'll modify this tomorrow.
comment:9 Changed 6 years ago by
 Commit changed from 371e40fe64f300e48ceeacc69f9d09405674b3a3 to 60da7a81e17caa667f0429b5a149d8fcfa9af1d5
Branch pushed to git repo; I updated commit sha1. New commits:
60da7a8  An example added as suggested by ncohen.

comment:10 Changed 6 years ago by
 Status changed from needs_work to needs_review
comment:11 Changed 6 years ago by
 Reviewers set to Nathann Cohen
 Status changed from needs_review to positive_review
THaaaaaaaaaaaanks ! Good to go :)
Nathann
comment:12 Changed 6 years ago by
 Branch changed from u/jmantysalo/warning_to_matrix_random__ to 60da7a81e17caa667f0429b5a149d8fcfa9af1d5
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
Added a warning.