Conversation
SuperRenormHP model
…ure) new file: yaml_files/CosmoBit_example_ALPs.yaml
modified: ../CosmoBit/src/CosmoALPs.cpp modified: ../CosmoBit/src/CosmoBit_types.cpp
modified: Backends/patches/classy/2.6.3/classy_2.6.3.diff modified: Backends/patches/classy/2.9.3/classy_2.9.3.diff modified: Backends/patches/classy/2.9.4/classy_2.9.4.diff
* Only keep XrayInterpolator (the twin sibling of AxionInterpolator) modified: DarkBit/src/Xray.cpp
modified: DarkBit/include/gambit/DarkBit/DarkBit_rollcall.hpp modified: DarkBit/src/Xray.cpp
modified: DarkBit/include/gambit/DarkBit/DarkBit_rollcall.hpp modified: DarkBit/src/Xray.cpp modified: config/capabilities.dat modified: yaml_files/Xray_test.yaml modified: yaml_files/Xray_test_2.yaml
modified: DarkBit/src/Xray.cpp
modified: HISTORY
…to CosmoBit_development
Conflicts: cmake/backends.cmake
* Restores the 'U' flag for the ar command * Force serial build rather than parallel (I had problems with the parallel build before) modified: cmake/backends.cmake
* copied the changes of fb20cfc over to the build of v3.1.0 modified: cmake/backends.cmake
modified: Backends/include/gambit/Backends/frontends/Acropolis_1_2_1.hpp modified: Backends/include/gambit/Backends/frontends/MontePythonLike_3_5_0.hpp modified: Backends/include/gambit/Backends/frontends/classy_3_1_0.hpp modified: Backends/patches/montepythonlike/MPLike_patch_script.py modified: Backends/src/frontends/Acropolis_1_2_1.cpp modified: Backends/src/frontends/MontePythonLike_3_5_0.cpp modified: Backends/src/frontends/classy_3_1_0.cpp modified: CosmoBit/include/gambit/CosmoBit/CosmoBit_rollcall.hpp modified: CosmoBit/include/gambit/CosmoBit/CosmoBit_types.hpp modified: CosmoBit/src/CosmoALPs.cpp modified: CosmoBit/src/CosmoBit_types.cpp modified: Models/include/gambit/Models/models/CosmoNuisanceModels.hpp modified: Models/include/gambit/Models/models/DMEFT.hpp modified: Models/src/models/CosmoNuisanceModels.cpp modified: cmake/backends.cmake modified: yaml_files/include/cosmo_nuisance_SpectralDistortions_fid.yaml modified: yaml_files/include/cosmo_nuisance_SpectralDistortions_flat.yaml
pstoecker
left a comment
pstoecker
left a comment
There was a problem hiding this comment.
I went through the PR. I have not checked any of the disabled code.
I have only a few minor remarks and one major one.
Let's start with the major one. I do not totally like the structure that we have with BBN_abundances and BBN_abundances_photodissociation as there are these conditional dependencies on the latter on whenever we consider an ALP (or any other decaying particle) and the logic is "Whenever there is a result from BBN_abundances_photodissociation" use this result otherwise fall back to BBN_abundances. this does not feel GAMBIT-y. I would suggest adding a dummy default implementation for BBN_abundances_photodissociation that will just put the results from BBN_abundances through without any modifications. Furthermore 'BBN_abundances_photodissociation' should be renamed BBN_abundances and what is now known as BBN_abundances should get another name.
To the minor remarks.
- There are a few yaml files, that should not be published (those for the super-renormalisable HP, the symmetron and test files for the X-ray likelihoods. However, it might be too early to delete them forever. I suggest deleting them in one single commit and once the PR is through and the code is published this commit is reverted such that the files can be used again in development.
- There are still a few functionalities left that can be disabled. A major one is a new function in the classy interface
class_get_tzthat returns the age of of the universe at a certain redshift. Inigo requested this for his X-ray likelihoods. So far I have only implemented this forclassy_exo_2.7.2but not the other ones. The question is whether we disable the functionality or whether we keep them without mentioning anything. Furthermore the question is whether the functionality should be added to all classy versions or only the newest one? - The data files for the various new functionalities of super-renormalisable HP, the symmetron, and X-ray likelihoods will be still published. Is this our intention? Or do we opt to delete them temporarily and restore them by reverting this once the code is published?
This already sums up the majority of my remarks.
As I am biased as being one of the main persons who wrote the modifications in this PR, it would be great if there is a second pair of eyes to check the PR.
|
Apparently the CI test for python3 fails. But it seems to fail while building |
It must be related to this PR; the test passes on master. No passy, no mergy 😜 |
Might be it doesn't actually fail, i.e. could just be that the URL for MO was down. I'll relaunch it and see if it passes. |
|
Yes, now it passes. There was probably some hick-up while downloading MO. |
|
I have made all the changes you suggested. Please have a look. I also made sure that in CB_dev the yaml files for the SuperRenormHP, symmetron and Xrays are kept. Please do not merge CB_dev into CB_Acropolis any more. |
modified: Backends/include/gambit/Backends/frontends/classy_3_1_0.hpp modified: Backends/include/gambit/Backends/frontends/classy_exo_2_7_2.hpp modified: Backends/patches/classy/3.1.0/classy_3.1.0.diff modified: Backends/patches/classy/exo_2.7.2/classy_exo_2.7.2.diff modified: Backends/src/frontends/classy_3_1_0.cpp modified: Backends/src/frontends/classy_exo_2_7_2.cpp
…dances_BBN" capabilities * Renamed "primordial_abundances_LCDM" to "primordial_abundances_no_photodissociation" and removed explicit model dependences here, as LCDM is too restrictive (only etaBBN will do as well if only BBN is considered) * Fixed function and capability names in the existing CosmoBit related yaml files * Added explanation for the new BBN functionalities to "CosmoBit_tutorial" modified: CosmoBit/include/gambit/CosmoBit/CosmoBit_rollcall.hpp modified: CosmoBit/src/BBN.cpp modified: yaml_files/CosmoBit_quickStart.yaml modified: yaml_files/CosmoBit_tutorial.yaml
modified: .gitignore
…4de221 modified: Backends/src/frontends/classy_2_6_3.cpp modified: Backends/src/frontends/classy_2_9_3.cpp modified: Backends/src/frontends/classy_2_9_4.cpp modified: Backends/src/frontends/classy_exo_2_7_2.cpp
modified: Backends/include/gambit/Backends/frontends/Acropolis_1_2_1.hpp modified: Backends/src/frontends/Acropolis_1_2_1.cpp modified: CosmoBit/include/gambit/CosmoBit/CosmoBit_rollcall.hpp modified: CosmoBit/src/BBN.cpp modified: config/capabilities.dat modified: yaml_files/CosmoBit_tutorial.yaml
|
Seeing as all my comments were addressed, most conversations are resolved, and the CI test passed I would approve this pull request. The only outstanding and unresolved conversation is about the changes made to the AlterBBN build in line 178 of |
|
All of the issues have been resolved, so I will merge this to master and get ready to release it as soon as the paper goes to the arxiv. |
7 participants
This is the PR associated with the CosmoALP project. Besides the stuff documented in the paper, this also ships some of the work in progress for the super-renormalisable HP model and the symmetron, but I disabled those models and module functions to avoid future merge clashes.
I've tagged @pstoecker to review, but I'd also like @williamjameshandley to review it if possible to remove bias, as Patrick wrote a lot of what's going into the merge.