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

don't save HaloField by default in high level functions #436

Closed
wants to merge 2 commits into from

Conversation

daviesje
Copy link
Contributor

@daviesje daviesje commented Oct 14, 2024

Since halo fields can get quite large, and since the classes HaloFIeld and PerturbHaloField hold similar information, we really only want to cache one most of the time. So I've changed the default behaviour in run_coeval and run_lightcone to only write PerturbHaloField. This could save up to ~10% time and halve the cache requirements.

  • _get_config_options is now only called on the lower-level structures, this was done to have more control over writing behaviour.
  • If hooks are passed or write is set to true or false, the behaviour is the same as before
  • if write==None (default) we write everything except for HaloField.

This also fixes an unrelated problem where we overpurged the ICs for the halofield since we need the velocity fields for perturb_halo_list.

In the longer term it might be a better idea to have the halo field perturbation be an in-place operation. Where we have subclasses of HaloField to indicate Lagrangian or Eulerian, which can point to the same backend arrays.

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.

This looks good, thanks @daviesje. BTW does this fix the "writing when it's not supposed to" issues we've been having, by moving the config into the function instead of as the default?

@daviesje
Copy link
Contributor Author

This looks good, thanks @daviesje. BTW does this fix the "writing when it's not supposed to" issues we've been having, by moving the config into the function instead of as the default?

I'm not sure, if hooks==None and write==False the first config call will set hooks == {}and pass in the default write == None since that keyword isn't in iokw. After that, since len({}) == 0, it shouldn't fill the hooks with the default write.

@daviesje daviesje linked an issue Oct 23, 2024 that may be closed by this pull request
@daviesje
Copy link
Contributor Author

daviesje commented Nov 5, 2024

Closing this PR as it creates issues with caching and dependence. To be revisited later

@daviesje daviesje closed this Nov 5, 2024
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.

Make writing of PerturbHaloField optional
2 participants