Opened 3 years ago

Last modified 10 days ago

#28523 new enhancement

Signature of random_element

Reported by: Rémi Owned by:
Priority: minor Milestone: sage-9.8
Component: user interface Keywords: signature
Cc: Samuel Lelièvre Merged in:
Authors: Reviewers:
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by Samuel Lelièvre)

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.

Change History (11)

comment:1 Changed 3 years ago by John Palmieri

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?

comment:2 Changed 3 years ago by Rémi

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

comment:3 Changed 3 years ago by Samuel Lelièvre

Cc: Samuel Lelièvre added
Description: modified (diff)

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.

comment:4 Changed 3 years ago by Erik Bray

Milestone: sage-8.9sage-9.1

Ticket retargeted after milestone closed

comment:5 Changed 2 years ago by Matthias Köppe

Milestone: sage-9.1sage-9.2

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

comment:6 Changed 2 years ago by Matthias Köppe

Milestone: sage-9.2sage-9.3

comment:7 Changed 18 months ago by Matthias Köppe

Milestone: sage-9.3sage-9.4

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

comment:8 Changed 13 months ago by Matthias Köppe

Milestone: sage-9.4sage-9.5

comment:9 Changed 10 months ago by Matthias Köppe

Milestone: sage-9.5sage-9.6

comment:10 Changed 5 months ago by Matthias Köppe

Milestone: sage-9.6sage-9.7

comment:11 Changed 10 days ago by Matthias Köppe

Milestone: sage-9.7sage-9.8
Note: See TracTickets for help on using tickets.