BradGreig / Hybrid21CM

1 stars 3 forks source link

RANDOM_SEED really should *not* be in cosmo_params! #7

Closed steven-murray closed 5 years ago

steven-murray commented 5 years ago

At the moment, RANDOM_SEED is in CosmoParams, and it has to be dealt with specially in every method (because you could consider two CosmoParams() instances to be equal even if their random seeds are not the same, or are "unset", but often you don't want to consider them equal).

The obvious solution to this is to extricate the random seed from cosmo params, and have it as its own parameter that is passed to initial_conditions (and wherever else it might be required). It is a special kind of parameter, so it should be treated as one.

BradGreig commented 5 years ago

Random seed is only necessary for initial conditions. However, random seed will need to be passed to all other functions to ensure the correct boxes (i.e. matching random seeds) are read in/stored.

I thought RANDOM_SEED was apart of the unique hash. How can two CosmoParams() instances be the same with different random seeds?

steven-murray commented 5 years ago

I agree random seed is only used for initial conditions. In fact I think it probably more naturally fits as an attribute of the initial conditions struct, rather than the cosmo params. Yes, as a standalone parameter, random_seed will have to be passed around between functions, which is a bit unfortunate, but is more explicit.

The ambiguity with random seed enters because if it is unset (i.e. RANDOM_SEED=None), then the behaviour is to consider the cosmo params equal to all cosmo params that have the same value for all other parameters, but don't necessarily match the seed. This is used to locate a box to read in from cache. Once that is read in, its stored seed is set as the actual seed, and it should no longer be considered equivalent to other cosmo_params with different seeds. This means that all the logic for the input structs has to be special-cased for the random seed all the time.

The other thing to think about is that random_seed is not really a parameter, in the sense that it does not change in a continuous way, like the other parameters of the cosmology. It is its own unique parameter that has its own behaviour.

steven-murray commented 5 years ago

This is changed in 3a16ed890762e57baf5bffa3688c0fbc3a5db3d5