# nondeterministic random number seeds in MeasurePsf candidate reservation

XMLWordPrintable

## Details

• Type: Bug
• Status: Done
• Resolution: Done
• Fix Version/s: None
• Component/s:
• Labels:
None
• Story Points:
1
• Epic Link:
• Team:
Data Release Production

## Description

MeasurePsfTask randomly reserves a fraction of its candidates for validation, in a way that is supposed to be deterministic. This seems to be broken; I've identified at least two problems:

• CharacterizeImageTask does not pass the expId argument to MeasurePsfTask.run, letting it default to zero.
• MeasurePsfTask uses Python's built-in random module instead of afw.math.Random. Contrary to its own documentation, calling random.seed(0) does not always produce deterministic results (though I've only been able to trigger this the first time I tried it):

 In [3]: random.seed(0)   In [4]: random.random() Out[4]: 0.7579544029403025   In [5]: random.seed(0)   In [6]: random.random() Out[6]: 0.8444218515250481   In [7]: random.seed(0)   In [8]: random.random() Out[8]: 0.8444218515250481   In [9]: random.seed(0)   In [10]: random.random() Out[10]: 0.8444218515250481 

I have no idea what could be going on in the built-in random module (and it's hard to report upstream since I was only able to trigger it once), but we should switch to our own random number generator regardless.

## Activity

Hide
Perry Gee added a comment -

I have been running a linter, but I did miss some things in the testPsfCandidateSelection.py. I cut-and-pasted this from the testPhotoCal, and guess I thought of the code as pre-existing when actually it was new.

The unnecessary import in photoCal.py was there already, and I have been told not to fix such things as I am changing other code.
I added this as an additional commit since you asked for it. I guess that is OK to do.

I will fix the commit structure after you review the code as it currently stands. I know this can be done with rebase, but I need to figure out how.

Show
Perry Gee added a comment - I have been running a linter, but I did miss some things in the testPsfCandidateSelection.py. I cut-and-pasted this from the testPhotoCal, and guess I thought of the code as pre-existing when actually it was new. The unnecessary import in photoCal.py was there already, and I have been told not to fix such things as I am changing other code. I added this as an additional commit since you asked for it. I guess that is OK to do. I will fix the commit structure after you review the code as it currently stands. I know this can be done with rebase, but I need to figure out how.
Hide
Perry Gee added a comment -

Lauren MacArthur I would like to finish up this ticket before I leave for good this week. I left it for you to finish the review when I took off for vacation in June, and it has been sitting every since, I guess.

I rebased it yesterday, and tried to answer your objections, but some are pretty general, so I'm not sure.

If you can't get to it, please let me know and I can ask John to take care of it.

Show
Perry Gee added a comment - Lauren MacArthur I would like to finish up this ticket before I leave for good this week. I left it for you to finish the review when I took off for vacation in June, and it has been sitting every since, I guess. I rebased it yesterday, and tried to answer your objections, but some are pretty general, so I'm not sure. If you can't get to it, please let me know and I can ask John to take care of it.
Hide
Lauren MacArthur added a comment -

Some final comments on the PR. My above comments about commit structure still stand, but my review is complete.

Show
Lauren MacArthur added a comment - Some final comments on the PR. My above comments about commit structure still stand, but my review is complete.
Hide
Lauren MacArthur added a comment -

Also, please don't forget to rebase to master and run Jenkins before merging.

Show
Lauren MacArthur added a comment - Also, please don't forget to rebase to master and run Jenkins before merging.
Hide
Perry Gee added a comment -

I have fixed all the minor problems Lauren flagged which still appeared in the code. Paul has asked me in the past not to comment in github on every edit, but John has also told me that all problems should be either fixed or the reviewer should be told why not. So I have chosen to comment here.

I fixed all the problems newly noted from Lauren's comments of 5 days ago, or those which she noted where still in the code from the previous review.

I am now running Jenkins, and will merge when that is done.

Show
Perry Gee added a comment - I have fixed all the minor problems Lauren flagged which still appeared in the code. Paul has asked me in the past not to comment in github on every edit, but John has also told me that all problems should be either fixed or the reviewer should be told why not. So I have chosen to comment here. I fixed all the problems newly noted from Lauren's comments of 5 days ago, or those which she noted where still in the code from the previous review. I am now running Jenkins, and will merge when that is done.

## People

• Assignee:
Perry Gee
Reporter:
Jim Bosch
Reviewers:
Lauren MacArthur
Watchers:
Jim Bosch, Lauren MacArthur, Perry Gee
• Votes:
0 Vote for this issue
Watchers:
3 Start watching this issue

## Dates

• Created:
Updated:
Resolved: