Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding ETHOS dark matter power spectrum to the code, plus smooth-k window function for making HMF #252

Draft
wants to merge 27 commits into
base: master
Choose a base branch
from

Conversation

charlottenosam
Copy link
Contributor

This PR adds the ETHOS DM power spectrum as a function of new AstroParams h_PEAK and log10_k_PEAK (see Bohr+2020, Mason+in prep).

This parameterized power spectrum spans WDM (h_PEAK=0) to CDM (log10_k_PEAK --> inf), and everything in between has dark acoustic oscillations. log10_k_PEAK [1-10] is the scale at which the matter power spectrum has its first oscillation (or cut-off for WDM) and h_PEAK [0-1] is the relative height of that oscillation. This should enable some easy explorations of a range of DM models.

Major code changes:

  • Add new AstroParams h_PEAK and log10_k_PEAK. These are really cosmology parameters but they won't effect ICs on large scales -- they are really only changing the HMF, so it's fine to treat them as AstroParams
  • Add USE_ETHOS flag to FlagOptions
  • Move HMF window function FILTER from GlobalParams to FlagOptions so that it can be easily changed after ICs are calculated
  • Add smooth-k window function for HMFs (see Leo+2018), which works better than tophat + sharp-k for truncated matter power spectra. The default parameters were calibrated to fit CDM, WDM and ETHOS DM models well at z=5-12.
  • Some broadcasting of these new variables to functions that need them when calculating HMFs etc.

I guess we don't include any example notebooks though - would you guys like a notebook before we do the PR?

@charlottenosam charlottenosam added context: python wrapper Predominantly affecting the Python wrapper context: C backend Changes occur predominantly in the C code type: feature: physical New physical feature to be included type: feature: code New ability in the code (user-side) labels Jul 26, 2021
@codecov
Copy link

codecov bot commented Jul 26, 2021

Codecov Report

Merging #252 (885b728) into master (7c81ba3) will decrease coverage by 0.14%.
The diff coverage is 71.42%.

@@            Coverage Diff             @@
##           master     #252      +/-   ##
==========================================
- Coverage   86.24%   86.10%   -0.15%     
==========================================
  Files          12       12              
  Lines        2778     2800      +22     
==========================================
+ Hits         2396     2411      +15     
- Misses        382      389       +7     
Impacted Files Coverage Δ
src/py21cmfast/inputs.py 87.61% <69.23%> (-2.28%) ⬇️
src/py21cmfast/outputs.py 92.13% <100.00%> (ø)

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 bb049dc...885b728. Read the comment docs.

Copy link
Member

@steven-murray steven-murray left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks so much @charlottenosam, this is really impressive.

I've made a couple of small comments on the code -- nothing major. The only major things to do, I think, are:

  • Update the changelog
  • Add a section to a tutorial (or a whole tutorial if you wish)
  • Add tests (perhaps just an extra test in the integration test suite)

src/py21cmfast/inputs.py Outdated Show resolved Hide resolved
h_PEAK, log10_k_PEAK: double, optional
ETHOS parameters for dark acoustic oscillations (DAOs) in the matter power spectrum.
Location (10^log10_k_PEAK h/Mpc-1) and height (h_PEAK from 0 to 1) of the first DAO peak.
Warm dark matter (WDM) corresponds to h_PEAK=0, and log10_k_PEAK changes its mass. See Ref.~YY. (TODO!)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just noting that there's a TODO here. Is there a plan to publish first then merge?

@@ -151,27 +151,30 @@ double HI_ion_crosssec(double nu);


/* R in Mpc, M in Msun */
double MtoR(double M){
double MtoR(double M, struct FlagOptions *flag_options){
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be cleaner to have MtoR depend on the int variable filter rather than all of flag_options.

Comment on lines +809 to +811
//add the suppression and DAOs due to ETHOS DM-DR interactions
// TODO: add debugging
// LOG_DEBUG("T=%e",ETHOS_DAOs(k, astro_params_sfrd));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noting another TODO here. remove if unnecessary.

@@ -4292,3 +4370,51 @@ void FreeTsInterpolationTables(struct FlagOptions *flag_options) {
LOG_DEBUG("Done Freeing interpolation table memory.");
interpolation_tables_allocated = false;
}

double ETHOS_DAOs(double k, struct AstroParams *astro_params_sfrd){
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

again, better to have it depend just on h_PEAK and log10_k_PEAK, then pass those in directly when calling (since it's just two variables) in my opinion.

Copy link
Member

@BradGreig BradGreig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @charlottenosam, this all looks pretty good to me.

My main comment basically extends the suggestion by @steven-murray. In passing through the couple extra parameters as function arguments, then you shouldn't need the Broadcast_struct_global_SFRD function.

src/py21cmfast/inputs.py Show resolved Hide resolved
@@ -190,6 +195,7 @@ int CreateFFTWWisdoms(struct UserParams *user_params, struct CosmoParams *cosmo_
void Broadcast_struct_global_PS(struct UserParams *user_params, struct CosmoParams *cosmo_params);
void Broadcast_struct_global_UF(struct UserParams *user_params, struct CosmoParams *cosmo_params);
void Broadcast_struct_global_HF(struct UserParams *user_params, struct CosmoParams *cosmo_params, struct AstroParams *astro_params, struct FlagOptions *flag_options);
void Broadcast_struct_global_SFRD(struct UserParams *user_params, struct CosmoParams *cosmo_params, struct AstroParams *astro_params, struct FlagOptions *flag_options);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you should remove this function, and follow @steven-murray's suggestion of just passing the FILTER, h_peak and log10_k_PEAK parameters explicitly to the functions expecting them.

The idea of these functions are to broadcast everything to all functions in the respective PS (ps.c), UF (UsefulFunctions.c) and HF (heating_helper_progs.c) files. Thus this is essentially a duplication of Broadcast_struct_global_PS. As you only need to pass a couple of specific arguments once or twice, explicitly adding those as function arguments would be preferable.

@charlottenosam charlottenosam marked this pull request as draft April 19, 2022 11:22
@charlottenosam
Copy link
Contributor Author

I am finally working on this again (slowly..!), so have converted to draft and will make the changes you have suggested :)

@steven-murray
Copy link
Member

@charlottenosam bumping this :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
context: C backend Changes occur predominantly in the C code context: python wrapper Predominantly affecting the Python wrapper type: feature: code New ability in the code (user-side) type: feature: physical New physical feature to be included
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants