Corrfunc icon indicating copy to clipboard operation
Corrfunc copied to clipboard

Custom r_pi (line-of-sight) binning

Open lgarrison opened this issue 7 years ago • 5 comments

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

lgarrison avatar Apr 04 '17 02:04 lgarrison

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

wagoner47 avatar Apr 12 '17 22:04 wagoner47

Thanks for the offer!

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

manodeep avatar Apr 13 '17 02:04 manodeep

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 [email protected] 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.

wagoner47 avatar Apr 13 '17 15:04 wagoner47

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:

  • Update the kernels to accept an array of pibins (e.g. countpairs_rp_pi_fallback_DOUBLE() in theory/DDrppi/countpairs_rp_pi_kernels.c.src). You can follow the existing logic for the rp bins. Note that this requires changing the call signature, so the internal API will be changing.
  • Update countpairs_rp_pi_DOUBLE() in theory/DDrppi/countpairs_rp_pi_impl.c.src to take a char *pibinfile argument instead of int pimax. You can follow the example of char *binfile.
  • Update the top-level C interface in the same way: countpairs_rp_pi() in theory/DDrppi/countpairs_rp_pi.c
  • Update the command line interfaces (theory/DDrppi/DDrppi.c) to take a pi bin file (again following the example of the rp bin file)
  • Update the Python C modules, e.g. countpairs_countpairs_rp_pi() in theory/python_bindings/_countpairs.c and the docstrings
  • Update the Python interfaces and docstrings, e.g. DDrppi() in Corrfunc/theory/DDrppi.py and the examples in Corrfunc/call_correlation_functions.py
  • Update the DDrppi tests: theory/tests/test_[non]periodic.c

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!

lgarrison avatar Apr 13 '17 16:04 lgarrison

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

manodeep avatar Apr 13 '17 23:04 manodeep