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

Remove/Improve Recursive Generation #363

Open
daviesje opened this issue Mar 27, 2024 · 1 comment · May be fixed by #415
Open

Remove/Improve Recursive Generation #363

daviesje opened this issue Mar 27, 2024 · 1 comment · May be fixed by #415
Assignees
Labels
API Breaking A change that breaks backwards compatibility context: python wrapper Predominantly affecting the Python wrapper context: v4-prep This issue regards changes to the v4-prep branch type: refactor Code refactoring / restyling for internal benefit

Comments

@daviesje
Copy link
Contributor

The current behaviour when we use boxes that require another redshift is to recursively generate all required snapshots until we get to the desired redshift.

There are some transparency issues with this since it can be hard to keep track of what redshifts are required and which you have already generated. But more importantly the addition of the stochastic halo fields (which need to be generated in the reverse order) as well as the XraySourceBox (which requires input fields from multiple redshifts) complicates this picture, making it very difficult to perform with recursion.

Two possibilities to simplify the picture are:

  • remove the recursion entirely, and force the user to either use the provided functions run_coeval/run_lightcone or to explicitly generate all the required snapshots.
  • replace the recursion with a more robust version which determines all the required redshifts before any calculation, then compares with the cache to determine which we already have and fetches/calculates the boxes in the right order.
@daviesje daviesje added context: python wrapper Predominantly affecting the Python wrapper context: v4-prep This issue regards changes to the v4-prep branch type: refactor Code refactoring / restyling for internal benefit labels Mar 27, 2024
@steven-murray steven-murray added the API Breaking A change that breaks backwards compatibility label Jul 3, 2024
@steven-murray
Copy link
Member

I prefer first option. Let's remove recursion and rely on the right boxes being passed in. We need to decide on how the progenitor and descendent redshifts are treated in terms of their effect on the box at hand (caching etc.) an how this interacts with a possible Z_HEAT_MAX

@steven-murray steven-murray self-assigned this Jul 3, 2024
@daviesje daviesje linked a pull request Oct 17, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Breaking A change that breaks backwards compatibility context: python wrapper Predominantly affecting the Python wrapper context: v4-prep This issue regards changes to the v4-prep branch type: refactor Code refactoring / restyling for internal benefit
Projects
Status: In progress
Development

Successfully merging a pull request may close this issue.

2 participants