Signature of random_element

[4322f910-9437-447e-8950-2a683c4c01c1]

The signature for the methods random_element of ZZ and RR are currently different.

More precisely if one wants to specify a lower bound and an upper bound for the random variable, the signatures are

ZZ.random_element(x=minValue, y=maxValue)
RR.random_element(min=minValue, max=maxValue)

The syntax for RR.random_element is rather confusing as well. For instance

RR.random_element(min=2)

returns a random number between 0 and 2, so 2 is the upper bound.

Finally, the documentation for random_matrix mentions the x and y parameters for random integer matrices without insisting on the fact that this is specific to integer matrices.

CC: @slel

Component: user interface

Keywords: signature

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

[jhpalmieri]

comment:1

It looks to me as though

RR.random_element(min=M)

returns a random number between 1 and M, not 0 and M. Well, with M=2, if it's between 1 and 2, then it's also between 0 and 2, but it should never actually be less than 1 in that case.

The method chooses a random number between 0 and 1 and then rescales it to lie between min (default -1) and max (default 1), regardless of which of min or max is larger. This is actually what the documentation says:

Return a uniformly distributed random number between ``min`` and ``max`` (default -1 to 1)

The names min and max are perhaps misleading.

I can see reasons for both the ZZ behavior and the RR behavior. What changes do you propose?

[4322f910-9437-447e-8950-2a683c4c01c1]

comment:2

I am rather new here, so I do not have rigid opinions. I see pro and con to both syntaxes. The important thing to me is that the signatures should be the same for all random_element methods.

If the choice is to use the keywords min and max then the default behaviors of random_element should probably be something like

RR.random_element(min=m)

renders a random element between m and some larger default (larger value)

RR.random_element(max=M)

renders a random element between some default (smaller) and M and

[slel]

Description changed:

--- 
+++ 
@@ -1,20 +1,18 @@
-It seems that depending on the ring we are using the signature of the method ramdom_element is different.
+The signature for the methods `random_element` of `ZZ` and `RR` are currently different.

-More precisely if one wants to specify a lower bound and an upper bound for the random variable, the signatures are
+More precisely if one wants to specify a lower bound and an upper bound
+for the random variable, the signatures are

-ZZ.random_element(x = minValue, y= maxValue) -RR.random_element(min = minValue, max = maxValue) +ZZ.random_element(x=minValue, y=maxValue) +RR.random_element(min=minValue, max=maxValue)

-
-The syntax for RR.random_element is rather confusing as well. For instance
+The syntax for `RR.random_element` is rather confusing as well. For instance

RR.random_element(min=2)

- 
+returns a random number between 0 and 2, so 2 is the upper bound.

-returns a random number between 0 and 2, so 2 is the upper bound
-
-
+Finally, the documentation for `random_matrix` mentions the `x` and `y` parameters for random integer matrices without insisting on the fact that this is specific to integer matrices.

[slel]

comment:3

I'm expanding the ticket description to mention random_matrix, since that was the starting point for Rémi's puzzlement, based on which I encouraged him to open a ticket.

The documentation for random_matrix should mention the fact that the extra arguments and keywords will be passed on to A.randomize() with A initialized as a zero matrix over the specified ring, and that these arguments will likely in turn be passed to ring.random_element for that ring. It should warn the user that different rings might require different arguments, even to perform similar functions, and it should invite the user to check the documentation for ring.random_element for the rings they need.

While the same arguments, min and max, make sense for picking an integer or a real at random, min and max might not suffice or make sense for all rings: for random rationals we may hope to set max denominator, for random polynomials max degree, etc.

[embray]

comment:4

Ticket retargeted after milestone closed

[mkoeppe]

comment:5

Moving tickets to milestone sage-9.2 based on a review of last modification date, branch status, and severity.

[mkoeppe]

comment:7

Sage development has entered the release candidate phase for 9.3. Setting a new milestone for this ticket based on a cursory review.