21cmfast / 21cmFAST

Official repository for 21cmFAST: a code for generating fast simulations of the cosmological 21cm signal
MIT License
58 stars 38 forks source link

Fix FFT normalisation in FindHaloes #267

Closed daviesje closed 2 years ago

daviesje commented 2 years ago

In the old version, the gaussian random field in k-space was passed to find_halos, which was normalised by VOLUME/TOT_NUM_PIXELS. Now the real field is passed to the function so the normalisation should be different after the iFFT.

BradGreig commented 2 years ago

Hey @daviesje indeed you are correct. It seems that when I was using my own test suite to implement this I just so happened to end up with the unfortunate case of having set BOX_LEN and DIM to the same number! Lesson learnt...

In order to get this to pass you'll need to update all the test data for any test containing USE_HALO_FIELD = True. You can see which tests these are via all the failed tests above. Once the data is updated and the tests pass this is good to be merged.

steven-murray commented 2 years ago

To create new test data, move to the tests/ directory and run

python produce_integration_test_data.py --names NAME --names ANOTHER_NAME ...

The names of the tests that you need to run, as @BradGreig said, are in the failing tests. You can use --help for more help.

daviesje commented 2 years ago

Thanks, I've regenerated the files used in the halo tests.

codecov[bot] commented 2 years ago

Codecov Report

Merging #267 (98817c9) into master (05e1d59) will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #267   +/-   ##
=======================================
  Coverage   86.24%   86.24%           
=======================================
  Files          12       12           
  Lines        2778     2778           
=======================================
  Hits         2396     2396           
  Misses        382      382           

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 5119fa5...98817c9. Read the comment docs.

BradGreig commented 2 years ago

Hey @steven-murray the docs are failing along with some of the plotting routines. Are you able to look into these, or bypass them?