sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.42k stars 477 forks source link

Doctests of random functions needs to be improved. #13555

Open d6c26cf6-9aec-430c-9f63-586ad566bb8c opened 12 years ago

d6c26cf6-9aec-430c-9f63-586ad566bb8c commented 12 years ago

Currently a lot of random functions are skipped during doc-testing, as their results can't be predicted.

However, in ticket #13554, it is clear that errors are slipping through, and properties of random objects should be checked when possible.

For example, in ticket #13554, the generated matrices should at least be tested for the following (if they aren't already):

Component: doctest coverage

Keywords: random, matrices, random_matrix, properties of random objects, properties

Issue created by migration from https://trac.sagemath.org/ticket/13555

d6c26cf6-9aec-430c-9f63-586ad566bb8c commented 12 years ago
comment:1

Sorry, on the last dot point from the documentation, it should check for the following instead:

1d7ec08f-60ae-4512-91a6-8324c06eab9f commented 12 years ago
comment:2

I think some of the desired tests are performed on the subsidiary methods. There was a desire at the time not to put too much in the main documentation.

Fir example, random_unimodular_matrix tests three very different results for determinant one.

d6c26cf6-9aec-430c-9f63-586ad566bb8c commented 12 years ago
comment:3

But if there had been some additional doctesting, the inconsistency between documentation and result would have been spotted a long time ago. The (duplicate) bug that inspired this trac report has been around since before 4.7.3.  #11968

If something goes into the documentation, then it should probably be tested there as well, or removed from the documentation. If a change somewhere else then breaks the doctest, then that at least acts as a flag that the documentation needs to be changed.

d6c26cf6-9aec-430c-9f63-586ad566bb8c commented 12 years ago
comment:4

This is some code I put together to check matrix values element by element:

A = random_matrix(ZZ,5); A
def checkfunc(matrix, func):
    for val in matrix.list():
        if func(val) == True:
            return True
    return False
checkfunc(A, lambda x: x == 0)

A = random_matrix(ZZ,5,x=4,y=10)
checkfunc(A, lambda x: (x >= 4 & x < 10))

It's been a while since I was an efficient python programmer, so I'm sure someone will show me a generator/list method which is a lot more efficient.

I did checkfunc as a function because I was thinking about large matricies and saving memory space (as well as returning on the first match to the conditions). I'm not satisfied with the fact that checkfunc iterates over a list rather than a generator.

jhpalmieri commented 12 years ago
comment:5

You might consider using all or any:

A = random_matrix(ZZ,5); A
def checkfunc(matrix, func):
    return any(func(val)==True for val in matrix.list()) # untested

Also, I'm not sure that there is an iterator for the elements of a matrix. I guess there's one for the rows (mat.__iter__()), and then I guess for each row there's one (row.iteritems()). I don't see one for all of the elements, though. (I tried searching the files in the matrix directory for "yield" and didn't find much.)

d6c26cf6-9aec-430c-9f63-586ad566bb8c commented 12 years ago
comment:6

I also got the second lambda function incorrect. We're testing if any values are outside the range 4 (inclusive) to 10 exclusive in my example matrix.

so it should be something like:

lambda x: (x < 4 | x >= 10)

or even better

lambda x: x not in range(4,10)

I did get an iterator-only method for getting elements of a matrix using this:

(A[valrow][valcolm] for valrow in xrange(A.dimensions()[0]) for valcolm in xrange(A.dimensions()[1]))

But then I realized that since we're only using this function (theoretically) in doc-checking, we probably don't have to worry about memory concerns and iterators versus lists. jhpalmieri's approach seems nice and simple.

d6c26cf6-9aec-430c-9f63-586ad566bb8c commented 12 years ago
comment:7

Theoretically we could use jhpalmieri's approach thus:

A = random_matrix(ZZ,5); A
any((lambda x: x == 0)(val)==True for val in A.list())

to completely avoid defining checkfunc at all. Don't know if this would be an advantage or not during doc-checking.

d6c26cf6-9aec-430c-9f63-586ad566bb8c commented 12 years ago
comment:8

Found a way to iterate over elements in a matrix using a generator.

if M is a matrix, then this:

(M[vals[0],vals[1]] for vals in xmrange(M.dimensions()))

will return a generator object that does the job.