Two input form of RANDOM procedure has odd behavior if first input is larger than second input.

[dmalec]

From the documentation:

With two inputs, RANDOM outputs a random integer greater than or equal to the first input, and less than or equal to the second input. Both inputs must be integers, and the first must be less than the second.

However, the procedure does not enforce that the first input is less than the second and behaves unexpectedly when the first input is greater than the second:

? PRINT (RANDOM 5 3)
825322468
?

I should have a pull request together for this shortly.

[brianharvey]

825322468

Hmm, are you on a 32-bit computer, or does the library use 32 bits even on a 64-bit computer?

[dmalec]

I'm running on a 64-bit computer; fair question on the 32-bit point, I'm not sure on that one.

[dmalec]

I do have a candidate PR that adds an error check for the condition; but, I'm also game to tackle this a different way if there's a better approach.

[brianharvey]

Nah, I'm sure it's fine... Just... When Berkeley got the contract to convert Unix from the (16-bit) PDP-11 to the (32-bit) Vax, all the random numbers became even, and it took like five years before anyone noticed. :( Something about bit shifting, I think? I forget.

[jrincayc]

After looking at the code, there seems to be another discrepancy between the documentation and the implementation. For the two input form, it just says that the inputs need to be "integers" but the code uses unsigned longs, so either the code should support signed integers, or the documentation should say the two input form needs non-negative integers. That is, should (RANDOM -10 -2) be valid?

[dmalec]

I'm a bit in favor of updating the code to support signed integers and allowing (RANDOM -10 -2). To be honest, writing some code where I attempted to use negative numbers for the range is what got me on this path :). I was playing around with changing the C code to allow that; but, then started wondering if that change would break existing Logo code that relied on large unsigned values as inputs.

[jrincayc]

Hm, my quick glance at csls didn't find anything that looked like it was using large unsigned values. Also, I don't think UBCLogo has a "Max unsigned int" available to users. I agree that something like ((1.0 * random :largeint) / :largeint) would be how to get a random number between 0 and 1 and so someone may have tried that. That however would be for the one argument form, so the two argument form is probably completely fine to switch to signed integers.

[brianharvey]

Yeah, I'm sure the intent was to allow negative integers. I wonder if the declaration is unsigned to match what the library provides.

Converting between integer-valued random numbers and real-valued ones is annoyingly complicated because the official grownup definitions are that integer ranges are closed [from, to] whereas reals are on the semi-closed interval [0, 1).

[dmalec]

Pulling up the code that generates the random number:

#ifdef HAVE_SRANDOM
        r = (range <= 0 ? 0 : random() % range);
#else
        r = (((long)rand()) << 15) | rand();
        r = (range <= 0 ? 0 : r % range);
#endif

https://github.com/jrincayc/ucblogo-code/blob/007bb0ba3f02a27ba0b6dbe4ca6a5222bf8110f0/math.c#L94,L99

I looked at the Open Group docs and I think the library functions should return signed values on systems that follow the standard:

long random(void);

https://pubs.opengroup.org/onlinepubs/9699919799/functions/random.html

int rand(void);

https://pubs.opengroup.org/onlinepubs/9699919799/functions/rand.html

That said, there might be implementations which return an unsigned value instead?

[dmalec]

I've updated the PR with a proposed change to allow negative numbers in the range form of RANDOM

[jrincayc]

@dmalec Thank you for fixing this.

[dmalec]

You're welcome, no worries :)