Skip to content

Conversation

AlexaBartlett
Copy link

I am creating a pull request at Boryana's recommendation because I have code that successfully implements ZCV for the tracer-matter cross-power spectrum. Doing so requires reading in the particle positions and adding them to the mock dictionary (example in abacus_ZCV_cross.ipynb). I have added flags to the functions that do this so as not to break existing code. If there are questions or concerns with the code as written, please reach out to me.

Comment on lines +186 to +187
matter_pos += Lbox / 2.0 # I think necessary for cross correlations
matter_pos %= Lbox
Copy link
Member

Choose a reason for hiding this comment

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

Is this shift matching something that the tracers are doing? Usually I try to wrap just the negative positions rather than shift all particles (since that changes the origin), but maybe the tracers are already being shifted somewhere?

Copy link
Author

Choose a reason for hiding this comment

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

This shift was matching something the tracers were doing, yes.

Copy link
Member

Choose a reason for hiding this comment

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

Oh I see, just a few lines above this the code was already doing tracer_pos += Lbox / 2.0.

@boryanah, do you know if this shift is necessary? The rest of the HOD module outputs in [-L/2, L/2) units (which you can see in, e.g. the galaxy file here).

@lgarrison
Copy link
Member

Cool, thanks! I'll let @boryanah review the science parts of the code, but could you make sure the tests are exercising the new code? Here is where we test the existing ZCV:

newBall.apply_zcv(mock_dict, config)

It's not actually testing the correctness of the outputs, which is something we should fix at some point. But if you could at least add another ZCV call that generates the cross-spectrum, that would be great.

It looks like the existing ZCV test is failing, too; could you take a look at that?

@lgarrison lgarrison requested a review from boryanah October 3, 2025 21:14
@lgarrison lgarrison added the ZCV Zel'dovich Control Variates module label Oct 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ZCV Zel'dovich Control Variates module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants