BUG fix galsim random seeding again #28
Conversation
|
@rmjarvis @sidneymau @esheldon please review! |
montara/des_tile.py
Outdated
|
|
||
| # launder through the RNG a few times to randomize | ||
| for _ in range(10): | ||
| first = galsim.BaseDeviate(first).raw() |
There was a problem hiding this comment.
"a few times" is silly. There's absolutely no reason to do it more than once.
There was a problem hiding this comment.
How does this fix the problem of the seeds?
| @@ -626,6 +626,10 @@ def setup(self, config, base, file_num, logger): | |||
| if not isinstance(rs, list): | |||
| first = galsim.config.ParseValue( | |||
| base['image'], 'random_seed', base, int)[0] | |||
|
|
|||
| # launder through the RNG to randomize | |||
| first = galsim.BaseDeviate(first).raw() | |||
There was a problem hiding this comment.
Can you explain how this fixes the seeding bug?
There was a problem hiding this comment.
Sure.
GalSim uses a different random number generator for each object being rendered. This is so the stamp rendering can be parallelized over multiple processes deterministically. The random seed for each object is sequential based on the initial random seed (first here).
The problem that Sid was running into was that he was seeding different exposures with not very different random seeds. They were chosen from a set of integers that were fairly small. <32,000 maybe? I'm not sure exactly. But not nearly enough to make the sequences of seed for each exposure be disjoint from the sequences for any other exposure.
This fix (and the similar one in GalSim here) updates the given random seed to map onto some other value in [1,2^64). This essentially guarantees that the sequence of seeds that are used for each object in the exposure will be unique relative to any other exposure. Or at least the probability that they aren't is down at 1/2^64, rather than 1/2^15.
There was a problem hiding this comment.
This sounds reasonable, but I think there might be something else going on in addition to whatever this is fixing.
The problem was way more severe than 1/2^15. Essentially the exact series of rotation values were used for objects in all simulated tiles. There seem to be a small fraction that mismatched between the lists, which is odd in itself.
For example there were 31281722 entries in the catalog I had from a large set of simulated tiles, but only 93233 unique rotation values. And when matt looked at the catalogs, most of them were in the same sequence, in order, for different images.
This is almost like the same initial seed is being used for each run of the code (although the claim is that this is not true). If something like that is going on, I think the above would not fix it, because this first = galsim.BaseDeviate(first).raw() would be going from the same initial state.
There was a problem hiding this comment.
I'm not definitively claiming that this is the only error. But it is a bug that we fixed in GalSim two years ago, which wasn't ported over to montara. It's possible there is something else wrong as well.
There was a problem hiding this comment.
Yeah I think this is likely the bug. The truth files record every time an object is drawn across all exposures and bands. The fact that the rotation values are only unique for a subset is the correct result.
Instead the issue is that the unique values end up repeated across different tiles. This happens because of the small seeds as Mike points out.
There was a problem hiding this comment.
My feeling is there is another issue as well, but we should go forward with this.
There was a problem hiding this comment.
Yep. My plan is to run some code at BNL to see what this fixes.
There was a problem hiding this comment.
One of the things that didn't make sense to me has been cleared up: I didn't realize the same object was in the truth catalog Nepoch times, which resulted in more duplicate rotation values than I would have predicted
There was a problem hiding this comment.
I was wondering about that, but hadn't thought about nepochs (only bands). Glad that is worked out!
This PR is repeat of the fix from Y3 for galsim seeding. That fix was put into the Y3 scripts to launch the sims and so did not make it into the new code base.
xref: https://github.com/des-science/y3-wl_image_sims/commit/04cbd46cbba9031c31f5d72388c1ec99e21101c0