Radio excess from ACG, MCG and PBH #297
Conversation
…e, Tools.py reads parameters from a ini file, *ini are inut files
…dded to TsBox to store T_Radio and previous snapshots
….c, JSC.h are some modular funcitons
…and some interpolation tables (Tables.h)
…and some interpolation tables (Tables.h)
…reverting back to the original inputs.py
…he original one (except for one test MSG)
…gnore all previous commits fot inputs.py (and possibly outputs.py)
…test version of output.py seems to be very different from the version I used, so I added caching tools for Radio Temp box and SFRD_box here manually
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Thank you @JSC42, this is a very cool set of features! I am far from understanding the code/physics, but I do have a few suggestions (other than the few comments I left). The two big things that I think need to be done in this PR are:
- The new stuff should all be made optional via one or more
FlagOptionswhich are by default set to False. - You should add one or more integration tests to the suite with these options turned on.
Something else to think about, maybe for @BradGreig / @qyx268: since there are so many parameters for this feature, would it be reasonable to admit another separate parameter struct? Eg. PBHParams? It would be annoying to implement this currently, because so many of the functions take in the four parameter structs, and this would have to be added everywhere. But this could also force us to be a bit more systematic in how we manage this, and open up the possibility of adding extensions more easily in the future.
Another thought: would it be possible to make the main part of this new calculation a new C function entirely, that takes in an IonizedBox and/or TsBox and computes the TradBox (optionally)? That would keep things a little more modular.
| for (i = 0; i < user_params->HII_DIM; i++) | ||
| { | ||
| for (j = 0; j < user_params->HII_DIM; j++) | ||
| { | ||
| for (k = 0; k < user_params->HII_DIM; k++) | ||
| { | ||
| *((float *)v + HII_R_FFT_INDEX(i, j, k)) = perturb_field->velocity[HII_R_INDEX(i, j, k)]; | ||
| } |
There was a problem hiding this comment.
Not sure I like this style here.
There was a problem hiding this comment.
FlagOptions are added for all the new features (USE_Radio_ACG, USE_Radio_MCG, USE_Radio_PBH, USE_Hawking_Radiation), defaults are set to False
There was a problem hiding this comment.
yeah I was using VS codes so the styling of BrightnessTemp.c is set by VS auto-indentation, anyway I will revert the code indentation style back to the original version
There was a problem hiding this comment.
I have reverted BrightnessTemperatureBox.c to the original indentation style
src/py21cmfast/src/JSC.h
Outdated
| // Debuging options, print out gas temp and SFRD box, I am gonna keep these | ||
| #define print_SFRD_box 1 | ||
| #define Reset_Radio_Temp_HMG 0 | ||
| #define debug_mode 1 |
There was a problem hiding this comment.
Might not need this since we have LOG_DEBUG etc.
There was a problem hiding this comment.
It might actually be more convenient to keep the print_SFRD_box feature, this is used to print out huge arrays to a file. Using other original debug features (e.g LOG_DEBUG) will also turn on a bunch of other debug infos that are irrelevant for the new features
| // Re-write of find_HII_bubbles.c for being accessible within the MCMC | ||
| // Note for JSC: Anything that begins with Radio or Hawking are my modules |
There was a problem hiding this comment.
maybe calling the module "JSC" isn't the best idea for the long-term. Could we call it something more meaningful?
There was a problem hiding this comment.
the JSC signatures in previous PR has now been removed
There was a problem hiding this comment.
and the JSC.h has also been renamed as RadioExcess.h
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #297 +/- ##
==========================================
- Coverage 86.53% 86.45% -0.09%
==========================================
Files 12 12
Lines 2807 2812 +5
==========================================
+ Hits 2429 2431 +2
- Misses 378 381 +3 ☔ View full report in Codecov by Sentry. |
|
@steven-murray yeah I agree. I'm also reviewing another work (although I'm not sure when if ever we will merge it into the master), which has introduced 11 new astro parameters and another 11 new flags... The developers were in fact including all astro parameters in UserParams, which py21cmmc cannot cope with. I agree a long term solution will be allowing new classes to be added for new and distinct features such as radio source / dark matter... For short term, they have to be put in astro_params if they are not affecting InitialConditions... |
…o need to keep txt print feature in io.c
…eedback for radio excess, default false
…ninitialised memory so an additional line is added to Ion.c
…inter can have mem issue that I cannot solve
Added excess radio background from: 1. Atomically cooled galaxies (ACG), 2. Molecularly cooled galaxies (MCG or minihalo), 3. Accreting PBH. Feature for Hawking radiation also included.