#1314 RandomNumbersTool.randomInt(int, int) might work incorrectly

[Tagir Valeev]

The method org.openscience.cdk.math.RandomNumbersTool.randomInt(int lo, int hi) (as of CDK 1.4.19) implements strange algorithm for generating random numbers so they are not evenly ditributed and in rare cases may lay outside of requested range.

The code reads as follow:
return (Math.abs(random.nextInt()) % (hi - lo + 1)) + lo;

The problem is Math.abs method which doesn't work correctly for Integer.MIN_VALUE (it will return the same value which is negative). On the other hand random.nextInt may produce this value (roughly one case in 4 billions), thus method may return the result which is less than lo.

How to reproduce (tested on OracleJDK 1.6.0.16 and OpenJDK 1.7.0.25):
RandomNumbersTool.setRandomSeed(0x260E92790EB7l);
System.out.println(RandomNumbersTool.randomInt(0, 10)); // -2

How to fix:
return random.nextInt(hi - lo + 1) + lo;

This bug was found by using static code analyzer FindBugs. FindBugs is the powerful tool to locate bugs and suspicious places in your Java code. It's free, installs easily, works quickly and brings a lot of fun! Consider analyzing the whole CDK code with FindBugs and you will find many interesting things!
http://findbugs.sourceforge.net/

[John May]

Hi Lany,

Thanks for the bug report will get that fixed. That test actually fails occasionally with 'was not in n% of a normal distribution'. I do already use find bugs for other projects but theres quite a few on the CDK to get through.

Many Thanks,
J

On 25 Oct 2013, at 08:39, Lany dzeguziite@users.sf.net wrote:

[bugs:#1314] RandomNumbersTool.randomInt(int, int) might work incorrectly

Status: open
Created: Fri Oct 25, 2013 07:39 AM UTC by Lany
Last Updated: Fri Oct 25, 2013 07:39 AM UTC
Owner: nobody

The method org.openscience.cdk.math.RandomNumbersTool.randomInt(int lo, int hi) (as of CDK 1.4.19) implements strange algorithm for generating random numbers so they are not evenly ditributed and in rare cases may lay outside of requested range.

The code reads as follow:
return (Math.abs(random.nextInt()) % (hi - lo + 1)) + lo;

The problem is Math.abs method which doesn't work correctly for Integer.MIN_VALUE (it will return the same value which is negative). On the other hand random.nextInt may produce this value (roughly one case in 4 billions), thus method may return the result which is less than lo.

How to reproduce (tested on OracleJDK 1.6.0.16 and OpenJDK 1.7.0.25):
RandomNumbersTool.setRandomSeed(0x260E92790EB7l);
System.out.println(RandomNumbersTool.randomInt(0, 10)); // -2

How to fix:
return random.nextInt(hi - lo + 1) + lo;

This bug was found by using static code analyzer FindBugs. FindBugs is the powerful tool to locate bugs and suspicious places in your Java code. It's free, installs easily, works quickly and brings a lot of fun! Consider analyzing the whole CDK code with FindBugs and you will find many interesting things!
http://findbugs.sourceforge.net/

Sent from sourceforge.net because you indicated interest in https://sourceforge.net/p/cdk/bugs/1314/

To unsubscribe from further messages, please visit https://sourceforge.net/auth/subscriptions/

[John May]

Patched - https://sourceforge.net/p/cdk/patches/701/