Lehman Garrison
Lehman Garrison
I've implemented the semantics from https://github.com/manodeep/Corrfunc/pull/276#discussion_r882985310. Specifically, a boxsize of -1 along any dimension now means that dimension is not periodic. The surface area of the change was pretty minimal;...
Thanks for the review; I think we've uncovered an unintended behavior in the binsize calculation predating this PR. Can you take a look at https://github.com/manodeep/Corrfunc/pull/276#discussion_r888974272 and see if you agree?
Hey @manodeep, can you take a look at this when you get the chance?
Great! The code now uses the spatial extent instead of the boxsize, and all the tests are passing, so I think we're ready to port to the other modules. Will...
@manodeep: xi and wp take a `double boxsize` argument in their C API, so they actually ignore `options->boxsize`. Do you think we should leave xi and wp alone in this...
I think I'd be okay with declaring that xi and wp only support cubic boxes. They are already restricted to periodic boxes, and we can point people to DD and...
Yes, exactly!
@johannesulf This PR should now support per-axis boxsize and periodicity for the other modules, including `DDsmu`. Would you like to check that `DDsmu` works as expected before we merge?
Excellent, thanks. @manodeep, good to merge?
I just checked, the crash still happens if the boxsize is passed (as Johannes' above example does). But this makes sense, because we changed the gridding to only span the...