Opened 5 years ago

Closed 5 years ago

#18177 closed defect (fixed)

Warning to Matrix.random()

Reported by: jmantysalo Owned by:
Priority: minor Milestone: sage-6.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 5 years ago by jmantysalo

  • Branch set to u/jmantysalo/warning_to_matrix_random__

comment:2 Changed 5 years ago by jmantysalo

  • Cc ​rbeezer added
  • Commit set to 371e40fe64f300e48ceeacc69f9d09405674b3a3
  • Status changed from new to needs_review

New commits:

371e40fAdded a warning.

comment:3 follow-up: Changed 5 years ago by 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))?

Nathann

comment:4 in reply to: ↑ 3 ; follow-up: Changed 5 years ago by jmantysalo

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 ; follow-up: Changed 5 years ago by ncohen

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?

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 5 years ago by jmantysalo

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 over GF(2) ^^;

Sorry, I meant 2x2 over GF(3). But the question still remains: is this useful for user?

comment:7 follow-up: Changed 5 years ago by 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>".

If you think that it is too much trouble, no problem.

comment:8 in reply to: ↑ 7 Changed 5 years ago by jmantysalo

  • 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 5 years ago by git

  • Commit changed from 371e40fe64f300e48ceeacc69f9d09405674b3a3 to 60da7a81e17caa667f0429b5a149d8fcfa9af1d5

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

60da7a8An example added as suggested by ncohen.

comment:10 Changed 5 years ago by jmantysalo

  • Status changed from needs_work to needs_review

comment:11 Changed 5 years ago by ncohen

  • Reviewers set to Nathann Cohen
  • Status changed from needs_review to positive_review

THaaaaaaaaaaaanks ! Good to go :-)

Nathann

comment:12 Changed 5 years ago by vbraun

  • Branch changed from u/jmantysalo/warning_to_matrix_random__ to 60da7a81e17caa667f0429b5a149d8fcfa9af1d5
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.