Skip to content

New theory object with contaminants #24

Draft
andreufont wants to merge 11 commits intomainfrom
new_theory
Draft

New theory object with contaminants #24
andreufont wants to merge 11 commits intomainfrom
new_theory

Conversation

@andreufont
Copy link
Contributor

Again, draft PR only to see the changes.

Not ready for public consumption yet.

Copy link
Collaborator

@marlena6 marlena6 left a comment

Choose a reason for hiding this comment

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

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.

@marlena6
Copy link
Collaborator

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?

@andreufont
Copy link
Contributor Author

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.

@andreufont
Copy link
Contributor Author

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).

@andreufont
Copy link
Contributor Author

andreufont commented Mar 17, 2026

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.

@andreufont
Copy link
Contributor Author

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 p3d(k, mu).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants