manodeep / Corrfunc

⚡️⚡️⚡️Blazing fast correlation functions on the CPU.
https://corrfunc.readthedocs.io
MIT License
164 stars 50 forks source link

Custom r_pi (line-of-sight) binning #116

Open lgarrison opened 7 years ago

lgarrison commented 7 years ago

Currently, DDrppi for both theory and mocks only supports unit binning in the line-of-sight direction via the pimax argument. We should change this to allow arbitrary binning as we do for r_p (i.e. read the bins from a file or from an array argument passed to the Python interface).

wagoner47 commented 7 years ago

I'd be interested in having this functionality. Is there any way I can help?

manodeep commented 7 years ago

Thanks for the offer!

What kind of functionality did you need - could you provide a bit more details about the custom binning strategy?

wagoner47 commented 7 years ago

So I’m looking to be able to compute \xi(r_p, pi). Because of that, I think the basic thing that might help would be to be able to also specify a pimin, so that it would at least be possible to run in a loop over parallel bins. But the idea in the original post with being able to provide bins in pi the same way as for r_p seems like the ideal case. I don’t know if binning in both directions could be handled the same way as just in r_p, though.

On Apr 12, 2017, at 7:06 PM, Manodeep Sinha notifications@github.com wrote:

Thanks for the offer!

What kind of functionality did you need - could you provide a bit more details about the custom binning strategy?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/manodeep/Corrfunc/issues/116#issuecomment-293756782, or mute the thread https://github.com/notifications/unsubscribe-auth/AKq1RwpBg0WyRTUPcuhvifHgybFOWBaWks5rvYNBgaJpZM4MyTPI.

lgarrison commented 7 years ago

I think this should be a fairly straightforward change, especially since nearly all the parts of the code that need to change can be found just by searching the codebase for pimax. The DDrppi modules already return 2D results arrays (of shape (N_rp_bins, N_pi_bins)), so that mechanism is already in place. I think the steps would be:

I used theory/DDrppi as an example, but mocks/DDrppi_mocks would need the same changes. I think the wtheta modules can stay as they are; those integrate the result up to pimax and return a 1D result.

Note that the AVX and SSE kernels would also have to be updated. If you haven't used AVX/SSE before, I'd start by updating the Fallback (i.e. plain C) kernels, and then deciding if you can figure out the AVX and SSE. If not, Manodeep or I could jump in and update those, since it would just be a reimplementation of the logic laid out in the Fallback kernel.

@manodeep, let me know what I missed!

manodeep commented 7 years ago

@lgarrison looks good. I don't think any of the combine pair counts sort of utilities will have to change.

I would simply change the internal API to be in arbitrary-but-contiguous pi-bins, similar to what already exists for rp-bins. However, I will add a functionality to the python wrappers, Corrfunc/theory/DDrppi.py and Corrfunc/mocks/DDrppi_mocks.py, to allow for a pimax or pi-bins-file. [And if you feel adventurous, then allow for (pimax, linear-binsize) as well with the default binsize being 1 Mpc/h.]

All of the existing python wrappers have a similar functionality for the rp-bins parameter. Internally, rp-bins is read-in from a file, but an array can be passed to any of the python wrapper (say Corrfunc/theory/DD.py). Use the utility Corrfunc.utils.return_file_with_rbins(binfile) to handle the conversion automatically between string/array to a suitable input for the Corrfunc API.