COIN-OR::LEMON - Graph Library

Opened 10 years ago

Closed 10 years ago

#173 closed defect (fixed)

rnd(1000) gives faulty result

Reported by: Alpar Juttner Owned by: Alpar Juttner
Priority: critical Milestone: LEMON 1.1 release
Component: core Version: release branch 1.0
Keywords: Cc:
Revision id: 4b2382fd80ef and r1.0

Description (last modified by Alpar Juttner)

If you call the operator() with an integer value, it will give faulty result (i.e. it is 0 all the time)

Attachments (1)

c4aa9f097ef1.patch (1.7 KB) - added by Alpar Juttner 10 years ago.

Download all attachments as: .zip

Change History (8)

Changed 10 years ago by Alpar Juttner

Attachment: c4aa9f097ef1.patch added

comment:1 Changed 10 years ago by Alpar Juttner

Description: modified (diff)
Priority: majorcritical
Status: newassigned

[c4aa9f097ef1] fixes this error by

  • changing using double as the parameters and return value of operator() instead of a template parameters.
  • I just removed real<Num>(Num) and real<Num>(Num,Num), as they suffer from the same problem and they are of no practical use.

Please review the path so that it could be merge to the main as well as the 1.0 branch.

comment:2 in reply to:  1 ; Changed 10 years ago by Peter Kovacs

Replying to alpar:

  • I just removed real<Num>(Num) and real<Num>(Num,Num), as they suffer from the same problem and they are of no practical use.

We could have something like that:

template <typename Number> 
Number operator()(Number b) { 
  return static_cast<Number>(real() * b);
}

Or we could remove these functions, as you did. It depends on what we want. Should rnd(1000) return a double or an int value? Should rnd(1000) be the same as rnd(1000.0), or they should differ?

comment:3 in reply to:  2 ; Changed 10 years ago by Alpar Juttner

Replying to kpeter:

We could have something like that:

template <typename Number> 
Number operator()(Number b) { 
  return static_cast<Number>(real() * b);
}

No, no, no! This would suffer from the same problem as the current version plus would introduce two more:

  • the generation of a float would slow down
  • the random long double would contain no more random bits than a random double

Or we could remove these functions, as you did. It depends on what we want. Should rnd(1000) return a double or an int value?

A double, of course, it must return a uniformly distributed real number from [0,1000). For generating integers from the interval [0..999] you must use rnd[1000].

Should rnd(1000) be the same as rnd(1000.0), or they should differ?

It should certainly be the same.

comment:4 in reply to:  3 ; Changed 10 years ago by Peter Kovacs

Replying to alpar:

No, no, no! This would suffer from the same problem as the current version

Why? I think not, because in the above solution the double --> int conversion would be preformed after the multiplaction, not before it.

plus would introduce two more:

  • the generation of a float would slow down
  • a random long double would contain no more random bits than a random double

It is true.

A double, of course, it must return a uniformly distributed real number from [0,1000). For generating integers from the interval [0..999] you must use rnd[1000].

Then your patch is the correct solution.

However I think it also suffers from the following problems:

  • the generation of a float would slow down
  • the random long double would contain no more random bits than a random double

comment:5 in reply to:  4 ; Changed 10 years ago by Alpar Juttner

Replying to kpeter:

Why? I think not, because in the above solution the double --> int conversion would be preformed after the multiplaction, not before it.

It is a bit better, but it still features the same problem: rnd(1000) gives something different (i.e. a random integer) from what would be expected (i.e. a random real floating point number).

Then your patch is the correct solution.

However I think it also suffers from the following problems:

  • the generation of a float would slow down
  • the random long double would contain no more random bits than a random double

No, because operation() is now obviously only for generating a double. It is clear that if you want to generate a long double (or a {{float}}} very fast), then you must use real<>().

comment:6 in reply to:  5 ; Changed 10 years ago by Peter Kovacs

Replying to alpar:

It is a bit better, but it still features the same problem: rnd(1000) gives something different (i.e. a random integer) from what would be expected (i.e. a random real floating point number).

Exactly. That's why I wrote at the beginning that the solution depends on what we want (in which I was not sure).

So I think your changeset should be merged into both branches.

comment:7 in reply to:  6 Changed 10 years ago by Alpar Juttner

Resolution: fixed
Status: assignedclosed

Replying to kpeter:

So I think your changeset should be merged into both branches.

Done.

Note: See TracTickets for help on using tickets.