21cmfast / 21cmFAST

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

The heating PR from debanjan, now a 21cmfast-native one #322

Closed steven-murray closed 1 year ago

steven-murray commented 1 year ago

This is the same PR, but we have a local branch of it now, so we can finish it off.

BradGreig commented 1 year ago

Updates on this:

I've made some minor changes to the interpolation table to ensure its more stable and doesn't exceed its boundaries. I've also added som memory cleanup for the new arrays.

The plots below are for log10(L_X) = 38., and appear to retain their original behaviour after the implementation of this. deltaTb.pdf PowerSpectra.pdf

Tests are still failing though. I'm pretty confident that the failing of most tests is just numerical (rounding errors), with the tests being a little too rigid for noticing that zero and 10^-60 are effectively equivalent.

The one test of concern is "ts_fluct_no_tables". The test data is quite discrepant, which I have yet to be able to rectify. I'm hoping it turns out to be as simple as the test data was generated from an older commit pre changes.

codecov[bot] commented 1 year ago

Codecov Report

Patch and project coverage have no change.

Comparison is base (58cfb1e) 86.50% compared to head (f98e698) 86.50%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #322 +/- ## ======================================= Coverage 86.50% 86.50% ======================================= Files 12 12 Lines 2809 2809 ======================================= Hits 2430 2430 Misses 379 379 ``` | [Impacted Files](https://app.codecov.io/gh/21cmfast/21cmFAST/pull/322?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=21cmfast) | Coverage Δ | | |---|---|---| | [src/py21cmfast/inputs.py](https://app.codecov.io/gh/21cmfast/21cmFAST/pull/322?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=21cmfast#diff-c3JjL3B5MjFjbWZhc3QvaW5wdXRzLnB5) | `90.30% <ø> (ø)` | |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

BradGreig commented 1 year ago

Hey @steven-murray, I think this PR is now ready. Not sure if this should go in before or after #320.

Additionally, this includes minor changes to address #325.

steven-murray commented 1 year ago

@BradGreig great, thanks! I'll have a look today, and also try to finalize #320 .

steven-murray commented 1 year ago

Adding my nominal approval to this PR (I can't approve since I made the PR technically).