Uploaded image for project: 'Request For Comments'
  1. Request For Comments
  2. RFC-436

Allow seed=0 for lsst.afw.math.Random

    Details

    • Type: RFC
    • Status: Withdrawn
    • Resolution: Done
    • Component/s: DM
    • Labels:
      None

      Description

      For a long time, the lsst.afw.math.Random constructor has disallowed seed=0: the doxygen says:

      Passing a seed-value of zero will cause the generator to be seeded with an algorithm specific default value.

      although passing seed=0 actually results in a exception.

      It is desirable to never use seed=0 when initialising the GSL random number generator underlying our implementation (because we want to know the seed we've used, which isn't possible when seed=0), but I don't think this restriction should be passed on to the user since it can be hidden, so it's an unnecessary burden on the user.

      I propose changing lsst.afw.math.Random to allow seed=0 to be provided by the user. The proposed code change results in changes to the mapping between the seed provided to lsst.afw.math.Random and the seed used in GSL, which means that old seeds we've recorded will not produce the same set of random numbers. While this will necessitate updating the values in the Demo, I don't anticipate any further problems because:

      • lsst.afw.math.Random is not used as often as numpy.random.
      • Besides the Demo, we don't rely on getting the exact same set of random numbers anywhere that I'm aware of.
      • If there is a need to get the exact same set of random numbers for some reason (e.g., comparing modern results to results obtained before the change for the sake of debugging), we can calculate the seed to provide to lsst.afw.math.Random that will produce the same GSL seed we used under the former scheme.

        Attachments

          Issue Links

            Activity

            Hide
            jbosch Jim Bosch added a comment -

            We do record the random number seeds we used when replacing neighbors with noise during measurement, in order to allow us to later reproduce the measurement results when debugging. We don't use that a lot right now, so it may be okay to make it impossible to use newer versions of the stack to reproduce measurement outputs currently on disk, but I believe Nate Lust is making use of that function on some reruns right now.

            Show
            jbosch Jim Bosch added a comment - We do record the random number seeds we used when replacing neighbors with noise during measurement, in order to allow us to later reproduce the measurement results when debugging. We don't use that a lot right now, so it may be okay to make it impossible to use newer versions of the stack to reproduce measurement outputs currently on disk, but I believe Nate Lust is making use of that function on some reruns right now.
            Hide
            rhl Robert Lupton added a comment -

            What is the motivation?  You should never use default values of seeds for randoms (you'll always regret it eventually, or, if it's production code, you'll always be cursed for it eventually)

             

             

            Show
            rhl Robert Lupton added a comment - What is the motivation?  You should never use default values of seeds for randoms (you'll always regret it eventually, or, if it's production code, you'll always be cursed for it eventually)    
            Hide
            price Paul Price added a comment -

            The motivation is to remove a minor user annoyance. In order to not use default values of the seed, we disallowed specifying seed=0, but this imposes an extra, unnecessary burden on the user. I am most certainly not proposing allowing default values of seeds.

            Show
            price Paul Price added a comment - The motivation is to remove a minor user annoyance. In order to not use default values of the seed, we disallowed specifying seed=0 , but this imposes an extra, unnecessary burden on the user. I am most certainly not proposing allowing default values of seeds.
            Hide
            rhl Robert Lupton added a comment -

            I don't understand.  You agree that the user should never use a unknown seed so they should never pass 0;  but yet you think that they should be allowed to?

             

            Show
            rhl Robert Lupton added a comment - I don't understand.  You agree that the user should never use a unknown seed so they should never pass 0;  but yet you think that they should be allowed to?  
            Hide
            price Paul Price added a comment -

            I don't think the user should be prohibited from using a particular value because that creates an unnecessary burden (I have to get a seed value, then check that it's not zero, fix it if it is, then pass it in). The code should convert a zero value passed to it to a non-zero value that it passes to GSL.

            Show
            price Paul Price added a comment - I don't think the user should be prohibited from using a particular value because that creates an unnecessary burden (I have to get a seed value, then check that it's not zero, fix it if it is, then pass it in). The code should convert a zero value passed to it to a non-zero value that it passes to GSL.
            Hide
            ktl Kian-Tat Lim added a comment -

            Is there any particular reason that all random seeds have to change just to allow zero to be specified?  Why not make zero into std::numeric_limits<unsigned long>::max(), for example, while leaving all others the same?  Or just turn zero into one?

            Show
            ktl Kian-Tat Lim added a comment - Is there any particular reason that all random seeds have to change just to allow zero to be specified?  Why not make zero into std::numeric_limits<unsigned long>::max() , for example, while leaving all others the same?  Or just turn zero into one?
            Hide
            jbosch Jim Bosch added a comment -

            I like Kian-Tat Lim's proposal. Sounds like it would solve the problem while being fully backwards compatible - I don't think we have any code that demands that different seed values produce different random numbers, so just mapping 0 to (e.g.) 1 should be fine.

            Show
            jbosch Jim Bosch added a comment - I like Kian-Tat Lim 's proposal. Sounds like it would solve the problem while being fully backwards compatible - I don't think we have any code that demands that different seed values produce different random numbers, so just mapping 0 to (e.g.) 1 should be fine.
            Hide
            price Paul Price added a comment -

            Will use Kian-Tat Lim's solution instead of my stupid one.

            Show
            price Paul Price added a comment - Will use Kian-Tat Lim 's solution instead of my stupid one.

              People

              • Assignee:
                price Paul Price
                Reporter:
                price Paul Price
                Watchers:
                Jim Bosch, John Swinbank, Kian-Tat Lim, Paul Price, Robert Lupton, Tim Jenness
              • Votes:
                0 Vote for this issue
                Watchers:
                6 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Planned End:

                  Summary Panel