duncandc / intrinsic_alignments

project to measure and model the intrinsic alignments of galaxies in both observations and simulations
2 stars 1 forks source link

stochasticity treatment #1

Closed aphearin closed 6 years ago

aphearin commented 6 years ago

@duncandc - Watch out for this pattern:

with NumpyRNGContext(seed):
    halo_axisA_x = np.random.random(len(table))*2-1
    halo_axisA_y = np.random.random(len(table))*2-1
    halo_axisA_z = np.random.random(len(table))*2-1

For cases where there are multiple calls to some np.random function within the NumpyRNGContext with the exact same arguments (in this case len(table)), this can lead to artificial correlations in the generated numbers. It's is safer to do:

with NumpyRNGContext(seed):
    halo_axisA_x = np.random.random(len(table))*2-1
with NumpyRNGContext(seed+1):
    halo_axisA_y = np.random.random(len(table))*2-1
with NumpyRNGContext(seed+2):
    halo_axisA_z = np.random.random(len(table))*2-1

I first noticed this in the anisotropic_nfw_phase_space.py module on lines 69-72, and 87-90 (line numbers apply to commit 6c2978d99ea71ff9d1197e617e15418541db156f). I don't know where else in the intrinsic_alignments repo this occurs, but it's worth running a recursive grep to find them: I remember this giving buggy behavior of satellites distributions in early versions of Halotools.

duncandc commented 6 years ago

This should be fixed by my last push.

duncandc commented 6 years ago

@aphearin hmmm, what do you do when seed is None?

duncandc commented 6 years ago

I could just do something like: if seed is not None: seed = seed + 1 before each with NumpyRNGContext(seed):

aphearin commented 6 years ago

That's what I do in this situation