GalSim
GalSim copied to clipboard
Update random_seed behavior in config to be less confusing.
@kailicao discovered an error in the recent Roman image simulations where the random seed was not updating for each galaxy as expected. This only affected Poisson noise on the flux values (the "realized flux"), since most values were from SkyCatalog, not generated from an rng. But the statistics of the fluxes ended up messed up.
@matroxel and I tracked down the problem to the random_seed specification in the config file, which used a dict to evaluate a value based on the input file. This was intended to be the initial seed, to be converted into a sequence as usual. But the (documented) behavior in that case is to take the user's dict specification as is, and not convert it. This is confusing behavior, since most of the time evaluation like this is equivalent to setting a literal value.
Troxel also noticed that imsim's example config has the same problem, although the config files used by @jchiang87 for the recent simulation runs didn't use that. (It used a string eval value for the seed.) So imsim users may also be getting wrong random values currently.
So I changed the behavior to always convert the first random_seed in the list (or only one if not a list) into the sequence in the usual way. Any other items in the list are evaluated as the user specifies. This is technically an API change, since it's possible that some users have been relying on the old behavior to leave dict specifications as is and convert int or str values. However, (a) I think it's unlikely that anyone was using this feature other than to have the first item be normal and later items be dicts, and (b) we know of two accidental erroneous usages mentioned above, and it wouldn't surprise me if there were others. So this is more like a bug fix than a feature change. Anyway, I document it as an API change nonetheless to warn users who might be relying on the old behavior.