manodeep / Corrfunc

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

boxsize when periodic = False #256

Closed adematti closed 2 years ago

adematti commented 2 years ago

General information

Issue description

Using Python wrapper for theory.DD (the same is true for e.g. DDsmu), not providing boxsize (as periodic=True) results in a RuntimeError (here: https://github.com/manodeep/Corrfunc/blob/cacc923ffc4e43dcde1b0f8a2dafe0493e0c4d74/Corrfunc/theory/DD.py#L256)

Expected behavior

No error raised.

What have you tried so far?

This issue is due to boxsize = None being passed by https://github.com/manodeep/Corrfunc/blob/cacc923ffc4e43dcde1b0f8a2dafe0493e0c4d74/Corrfunc/theory/DD.py#L240 to the CPython code in https://github.com/manodeep/Corrfunc/blob/cacc923ffc4e43dcde1b0f8a2dafe0493e0c4d74/theory/python_bindings/_countpairs.c#L1187 when "d" in PyArg_ParseTupleAndKeywords requires a Python double (not None). Simply setting boxsize to e.g. 1.0 if None before https://github.com/manodeep/Corrfunc/blob/cacc923ffc4e43dcde1b0f8a2dafe0493e0c4d74/Corrfunc/theory/DD.py#L240 solves the issue.

lgarrison commented 2 years ago

Oops, good catch, thanks for the report!

manodeep commented 2 years ago

Many thanks for the report and thanks @lgarrison for the fix (in #257) too!