Cleaning the turnover mess - #737
Conversation
…iables when USE_MINI_HALOS=False. In addition, changed a bit the logic of the MCG lower turnover mass in IonisationBox.c for SOUCE_MODEL=E-INTEGRAL (by removing the dependency on mturn_m_nofb), to match it better with the logic used for other source model. All tests pass at this stage
…URN_STELLAR_FEEDBACK and the output of atomic_cooling_threshold, even when USE_MINI_HALOS=False. Almost all tests passed since we normally use the default value for M_TURN_STELLAR_FEEDBACK, 8.7, which is greater than the typical atomic cooling threshold. One single test didn’t pass and had to be modified, by introducing a new function called get_atomic_cooling_mass_threshold in cfuncs.py. Now all the tests pass
…replace acg_thresh. As a consequence, all calls to Nion_General_MINI were had to be updated by passing the ‘correct' ACG turnover mass, including EvaluateNionTs_MINI and EvaluateSFRD_MINI, as well as all functions that calls these functions (this includes some functions in heating_helper_progs.c, since the tau_X integrand calls EvaluateNionTs_MINI). As a consequence, also the pythonic functions evaluate_SFRD_z and evaluate_Nion_z had to be modified in order to pass the ‘correct’ ACG turnover mass. In the latter function, a new boolean flag called avoid_recursion was added in order to avoid infinite recursion (this new flag will be removed when USE_REIONIZATION_FEEDBACK is added). Finally, tests in test_c_interpolation_tables.py were updated to be able to pass at z < 10. After all these changes all non-integration tests now pass, the integration tests will be examined when acg_thresh will be removed from codespace
…alM. As a consequence, all calls to these functions had to be updated by passing the ‘correct’ ACG turnover mass, including EvaluateXray_Conditional, as well as all the functions that call EvaluateXray_Conditional. As a result, also the pythonic function evaluate_Xray_cond had to be modified in order to pass the ‘correct’ ACG turnover mass. Also changed the condition to zero the MCG contribution to include the scenario mtrun_MCG = mturn_ACG. After all these changes all non-integration tests now pass, the integration tests will be examined when acg_thresh will be removed from codespace
…t to replace acg_thresh. As a consequence, all calls to Nion_ConditionalM_MINI were had to be updated by passing the ‘correct' ACG turnover mass, including EvaluateNion_Conditional_MINI and EvaluateSFRD_Conditional_MINI, as well as all functions that call these functions. As a result, also the pythonic functions evaluate_SFRD_cond and evaluate_Nion_cond had to be modified in order to pass the ‘correct’ ACG turnover mass. After all these changes all tests now pass, including the integration tests: the integration tests keep passing (without changing any file in the database) because were careful in setting M_TURN_STELLAR_FEEDBACK=5 when USE_MINI_HALOS=True, and also since we do our tests at z=18, which is too high for the reionization feedback to become dominant
…the integration tests: the integration tests keep passing (without changing any file in the database) because we were careful in setting M_TURN_STELLAR_FEEDBACK=5 when USE_MINI_HALOS=True, and also since we do our tests at z=18, which is too high for the reionization feedback to become dominant
…te_mcrit_boxes in IonisationBox.c similar as much as possible to compute_mturns in thermochem.c. Also made some semantic changes to make it clearer that variables represent the log10 of Mturn, and not Mturn itself. In addition, the logic for determining the turnover masses in IonisationBox.c is exactly the same as in HaloBox.c (while SpinTemeratureBox.c lacks the reionization feedback). After this change, all test continue to pass, including integration tests, because we were careful in setting M_TURN_STELLAR_FEEDBACK=5 when USE_MINI_HALOS=True
…an accept either ‘NONE’, ‘ACG’, ‘MCG’ or ‘BOTH’. We now use this enum to control whether the reionization feedback is applied either on the ACG or MCG turnover masses (except in SpinTemperatureBox.c where the reionization feedback cannot be evaluated because the previous IonizedBox is not passed into that module). This enum helps to replace the previous logic that the reionization feedback is ALWAYS applied on BOTH the ACG and MCG turnover masses when USE_MINI_HALOS=True, while it is NEVER applied when USE_MINI_HALOS=False. The default of REIONIZATION_FEEDBACK_MODEL is designed to mimic the previous logic, based on the value of USE_MINI_HALOS. The logic in the C code was designed that the inhomogeneous ACG turnover mass is evaluated only when REIONIZATION_FEEDBACK_MODEL is either ‘ACG’ or ‘BOTH’, while the for other configurations the ACG turnover mass is assumed to be homogeneous and possibly depend only on redshift. Unfortunately, due to an unrelated bug, the inhomogeneous ACG turnover mass needs to be evaluated also when USE_MINI_HALOS=True and we use hmf interpolation tables (this was reported as issue 732 on github). All tests pass, including integration tests, since we run these tests with the same configuration and logic as before. One test in test_halo_sampler.py passes only when the reionization feedback is explicitly set on either ‘ACG’ or ‘BOTH’, namely the ACG turnover mass must be inhomogeneous in order for the test to work. I don’t know why, but I am not sure it’s due to this commit
…funcs.py into a new function called get_log10mturns_helper (2) removal of avoid_recursion flag in evaluate_Nion_z, (3) renaming lyman_werner_threshold to molecular_cooling_threshold_with_feedbacks and introducing a new pythonic function with a similar name, (4) renaming variables in IonisationBox.c, e.g. log10_Mturnover to log10_mturn_a and log10_Mturnover_MINI to log10_mturn_m. In addition, in EvaluateNion_Conditional we always use the input log10Mturn if we don’t use interpolation tables
…ired changing the signature of the function. Also changed the array inputs from float to double (no reason to have low-precision in this function, the arrays it contains are not huge. Finally, also renamed some variables in compute_luminosity_function to highlight that they correspond to ACGs or MCGs
…on tables for ACGs if the reionization feedback is applied on their turnover mass. This replaces the older logic that the ACG interpolation table was always (never) 2D when USE_MINI_HALOS = True (False). Also renamed several variables to make it clear that they refer to ACGs or MCGs. Finally, added new scenarios to test in test_Nion_conditional_tables by introducing reionization_feedback_model as a new parameter
…erate log10_mturn_a_grid if USE_MINI_HALOS=True and we use hmf interpolation tables, this is only required now if we apply the reionization feedback on the ACG turnover mass. (2) Prepared to pass an inhomogeneous ACG turnover mass in SpinTemperatureBox.c by changing the signature of functions and passing mturn_a_nofb (we cannot pass a fluctuating ACG turnover mass because we cannot use the reionization feedback in that module due issue #470, this is the reason why in EvaluateNionTs we can only use 1D interpolation table at the moment). All tests pass
…itialise_Nion_Conditional_spline and EvaluateSFRD_Conditional with EvaluateNion_Conditional. Most crucially, EvaluateSFRD_Conditional receives now log10Mturn_a as an input and can perform 2D interpolation if we apply the reionization feedback on the ACG turnover mass. Also added a few more tests. All tests pass
…on_Ts_spline and EvaluateSFRD with EvaluateNionTs. Most crucially, EvaluateSFRD receives now log10Mturn_ACG_ave as an input, though we still always pass there mturn_a_no_fb due to issue 470
… apply the reionization feedback on the ACG turnover mass, even when USE_MINI_HALOS=False. Also added more tests. All tests pass
We use now a new function called compute_mturns_inhomogeneous where our inhomogeneous turnover mass model is defined for both ACGs and MCGs, and it should be defined only in that function for consistency. I also changed the types of some variables from double to float, though I’m not sure it’s really necessary for all of them. In addition, I renamed mturn_a_nofb to mturn_acg_homogeneous for clarity. All tests pass
jordanflitter
added
type: bug
jordanflitter
added
the
type: feature: ui
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @jordanflitter, your pull request is larger than the review limit of 150000 diff characters
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## release-v4.3 #737 +/- ##
===============================================
Coverage ? 91.46%
===============================================
Files ? 33
Lines ? 5214
Branches ? 911
===============================================
Hits ? 4769
Misses ? 285
Partials ? 160 ☔ View full report in Codecov by Harness. |
…fixed one line in classy_interface.py to make it work with astropy v8
steven-murray
left a comment
steven-murray
left a comment
There was a problem hiding this comment.
This is really great, thank you so much @jordanflitter.
I really like the new document outlining the whole M_TURN evolution over releases, well done!
One thing I would say is that we should make it clear that f_duty should be read as the probability of a halo of mass m_h hosting a galaxy (of either ACG or MCG type). The explicit reason that the MCG's turnover for m>M_TURN_ACG is because it is more probable for them to form ACGs than MCGs. We should also make sure that f_ACG+f_MCG is never greater than unity...
| if (astro_options_global->USE_MINI_HALOS) { | ||
| for (j = 0; j < NMTURN; j++) { | ||
| // NOTE: we use below homogeneous (feedback-free) ACG turnover mass, because if | ||
| // the reionization reionization feedback dominates, then the the turnover |
There was a problem hiding this comment.
| // the reionization reionization feedback dominates, then the the turnover | |
| // the reionization feedback dominates, then the turnover |
| // We Force QAG due to the changing limits and messy implementation which I will fix | ||
| // later (hopefully move the whole thing to python) | ||
| if (source_model_is_mass_dependent(matter_options_global->SOURCE_MODEL)) { | ||
| // TODO: the passed ACG turnover mass that we give below has to be updated to |
There was a problem hiding this comment.
Flagging this as a thign we ought to actually do
There was a problem hiding this comment.
We agreed to keep that comment there, because the photon non-conservation correction will be completely rewritten in the future (#364)
|
Quick update: It is true that the probability of hosting a galaxy (whether MCG or ACG) is never greater than unity in this model. Furthermore, the conditional probability of hosting an MCG, given that a galaxy is hosted at all, in this model is When Mmcg=Macg, this has the form So I think this model makes sense. The only thing that probably doesn't really make sense is to force there to be no MCG's when Mmcg>Macg. Without forcing it, the number of MCG's will naturally die off exponentially as Mmcg grows above Macg anyway, but will do so smoothly. |
|
Thanks @steven-murray for your comments, I made some changes according to them. From what I was able to tell, I think you are satisfied with how we model the duty fraction for both ACGs and MCGs in the code, but you'd like to remove the constraint that Plus, before I make any changes in the code's logic (and update the docs), we should see what @andreimesinger thinks, just to be sure that we are all on the same page. |
Commented about the approximation we make when we model the MCG duty fraction with a sharp cutoff at M_MCG > M_ACG
2 participants
Fix #729 and #732.
This PR changes the following:
M_TURNwithM_TURN_STELLAR_FEEDBACKand madeM_TURNas a deprecated parameter with the appropriate deprecation warning.REIONIZATION_FEEDBACK_MODELwith the options (1)"NONE", (2)"ACG", (3)"MCG"and (4)"BOTH". This new enum helps in breaking the coupling betweenUSE_MINI_HALOSand the application of the reionization feedback on the ACG and MCG turnover masses.lyman_werner_thresholdtomolecular_cooling_threshold_with_feedbacksandmturn_a_nofbtomturn_acg_homogeneous.M_TURN_STELLAR_FEEDBACK, (2) output ofatomic_cooling_thresholdand (3)reionization_feedback(when applicable). This is now the logic in all source models, exceptCONST-ION-EFF, and regardless the value ofUSE_MINI_HALOS. An exception still occurs forE-INTEGRALinSpinTemperatureBox.c, where the reionization feedback is not applied due to Reionization feedback is ignored inSpinTemperatureBox.cwhenSOURCE_MODELbeing"E-INTEGRAL"or"CONST-ION-EFF"#470.M_TURN_STELLAR_FEEDBACK, (2) output ofmolecular_cooling_threshold_with_feedbacksand (3)reionization_feedback(when applicable). This is now the logic in all source models, exceptCONST-ION-EFF, and regardless the value ofUSE_MINI_HALOS. An exception still occurs forE-INTEGRALinSpinTemperatureBox.c, where the reionization feedback is not applied due to Reionization feedback is ignored inSpinTemperatureBox.cwhenSOURCE_MODELbeing"E-INTEGRAL"or"CONST-ION-EFF"#470.USE_MINI_HALOS), their turnover mass is homogeneous and is taken frommturn_acg_homogeneous, which is now always the maximum between (1)M_TURN_STELLAR_FEEDBACK, and (2) output ofatomic_cooling_threshold. The MCG turnover mass is always inhomogeneous.compute_mturns_inhomogeneousinthermochem.c(exception still occurs forE-INTEGRALinSpinTemperatureBox.cdue to Reionization feedback is ignored inSpinTemperatureBox.cwhenSOURCE_MODELbeing"E-INTEGRAL"or"CONST-ION-EFF"#470).ComuteLFto reflect the above changes in the turnover mass logic.EvaluateSomething(conditional/unconditional SFRD/Nion) to follow the above logic as well as the initialization of the interpolation tables.cfuncs.py, the following functions were added:get_atomic_cooling_mass_threshold,get_molecular_cooling_threshold_with_feedbacksandget_reionization_feedback. These are helpful functions for testing that the way the turnover masses are computed in the C code follow the exact same logic that is described above (intest_cfuncs.py::test_compute_mturns_model, this test would fail if we ever change the turnover masses model in the C code).cfuncs.pyinto a new function calledget_log10mturns_helper.test_c_interpolation_tables.cand added a few more, due to the introduction ofREIONIZATION_FEEDBACK_MODELin the code.21cmFAST. This review is found in a new markdown file calledM_TURN.rstand is referred in the docs fromupdates_from_v3.rst. The latter markdown file was also modified to include other changes that were made between v3 and v4, and is now also mentioned inmisc.rstunder a new section called "I get different results compared to older versions of 21cmFAST, why?".Note that all the integration tests keep passing without changing the referenced data files. This is because in our tests:
M_TURN_STELLAR_FEEDBACK=8.7whenUSE_MINI_HALOS=False. This is a very high value, much higher than the atomic cooling threshold.M_TURN_STELLAR_FEEDBACK=5whenUSE_MINI_HALOS=True, so the molecular cooling threshold with feedbacks is always more dominant in our tests in this configuration.So despite all the above changes in the logics, our tests keep using effectively the same turnover mass as before.