DM-13615: TestDataset allows randomness #108
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jdswinbank
commented
Feb 22, 2018
python/lsst/meas/base/tests.py
Outdated
| @@ -197,9 +197,6 @@ def makePerturbedWcs(oldWcs, minScaleFactor=1.2, maxScaleFactor=1.5, | |||
| The default range for rotation is 30-60 degrees, and the default range for reference shift | |||
| is 0.5-1.0 arcseconds (these cannot be safely included directly as default values because Angle | |||
| objects are mutable). | |||
|
|
|||
| The random number generator is primed with the seed given. If ``None``, a seed is | |||
| automatically chosen. | |||
There was a problem hiding this comment.
This is still true, isn't it? I understand that you don't want randomSeed=None in general use, and I agree with defaulting it to something else, but that doesn't invalidate the statement you're removing.
TestDataset contains two methods (makePerturbedWcs and realize) that take an optional seed for the random number generator. In both cases, the seed defaults to None, which means the RNG is seeded in some non-deterministic manner. However, this is intended for tests, which we require are deterministic. The current API therefore makes it too easy (simply neglecting to specify the optional randomSeed keyword argument) to do something bad. Instead, default to a preset (and therefore deterministic) value for the seed. If the user wants things to change between invocations, he can specify different seeds.
Wasn't specifying an RNG seed.
PaulPrice
force-pushed
the
tickets/DM-13615
branch
from
d8167a9
to
5f4e98e
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.