Conversation
marlena6
left a comment
There was a problem hiding this comment.
Hi Andreu, I'm reading through and trying to understand the updates. Do your theory.py and model_lya.py replace lya_theory.py and lyaP3D.py? Or is the plan to make them all interact? I see some functions here with new functionality for contaminants, but some other parts seem to repeat existing functionality, so I'm wondering if the idea is I take your new files and integrate them with the old functions, or if you're planning to do that integration, or if this is written to supersede absolutely everything in lya_theory.py and lyaP3D.py? For example, I see an essentially empty get_default_igm_params in model_lya, which is existing functionality in lya_theory, so I guess you've left some things as dummy functions because you haven't yet integrated the existing code? But then it looks like you've rewritten the step of emulating the Lya params, although it's not clear to me why that couldn't have been done from the earlier code? Maybe this will be easier to clarify once we get a chance to talk in person; for now I'll keep working on testing the minimizer and fitting, etc, from my own branch.
|
Ok, looking through it more I guess a lot of this is still experimentation? If I'm reading it right, the model used to get the Lya + metals P3D only uses large scales, without Arinyo corrections, at the moment? Also, I see you've taken out most or all of the uses of the ForestFlow P3D function, is that purposeful or just for testing? |
|
At this point it is mostly experimentation, we can decide on Friday what the next steps should be. But yes, the two new modules would substitute the existing ones (note that forestflow_emu is not needed either). Before the discussion on Friday, however, it would be very useful if you had a look at the Appendix A in Overleaf, since writing that is what triggered me to take a shot at coding these up after seeing the the current implementation in the main branch had some limitations. |
|
Regarding whether to have or not the Arinyo model applied to the Lya x Silicon, this is a decision that we'll have to take at some point, but that it would be trivial to add to the code. The same applies to the Lya x HCD correlation, it is not clear whether this one should also have the Arinyo component to the Lya field, or just the Kaiser one. In the long term, the studies by Lucía and Daniel on HCD and metals will help us decide, but meanwhile we should just use one or the other (they should have almost no impact to the DR2 fits, I think). |
|
An example of limitations in the current implementation is that it was using P3D functions from ForestFlow, that didn't know about HCDs or metals. I did recycle the Px functionalities like PX_Mpc_detailed, but not the ArinyoModel since it wasn't very useful once you take into account contaminants. |
|
Regarding Px_Mpc_detailed, I might open an issue to make it even more basic, without p3d_params or new_cosmo_params variables that are now required, and without z. Really it should only require a function that returns |
Again, draft PR only to see the changes.
Not ready for public consumption yet.