desihub / desisim

DESI simulations
BSD 3-Clause "New" or "Revised" License
16 stars 22 forks source link

suspiciously low ELG/LRG/QSO flux #549

Closed sbailey closed 2 years ago

sbailey commented 3 years ago

This is a PR to try to study the suspiciously low ELG/LRG/QSO flux problem (#507 et al), which suspiciously seems to be happening more frequently nowadays.

coveralls commented 3 years ago

Coverage Status

Coverage increased (+0.02%) to 45.379% when pulling 1ac33280221b2c3c3aef7024980937b96b01f5a3 on elgflux into ca0a234753af57eb0fed83edfa0dab999de8d1c1 on master.

weaverba137 commented 3 years ago

@sbailey, the failing tests do record a random seed. What if you deliberately set that seed, so you can at least see what values are being generated by that seed?

sbailey commented 3 years ago

@moustakas could you take a look at this "suspiciously low ... flux" false alarm again, using this PR as the sandbox if needed? The original point of this test was to catch units interpretation errors for whether to include a 1e-17 term or not (we tripped on that multiple times years ago), but it is raising a false alarm so often that it is problematic for unrelated pull requests.

Critically: I tried hardcoding the random seed reported by the test and rerunning at NERSC and it didn't fail, but I haven't dug deeper for whether we have a bug in using the seed or whether we have a portability issue (e.g. different versions of numpy resulting in different sequences for the same seed).

moustakas commented 2 years ago

Superseded by #559; closing without merging.