manodeep / Corrfunc

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

DD(s,mu) function for mocks/theory #130

Closed nickhand closed 7 years ago

nickhand commented 7 years ago

This adds a pair counter for mocks as a function of s and mu that largely follows the DD(rp,pi) setup. I am sure there additional optimizations to be made but it seems like a good starting place.

This partly addresses #103.

I have tested the code against kdcount and they give the same answers. A data file and test script are available here: test_DDsmu.zip

manodeep commented 7 years ago

Thanks a bunch for sending this in. The broad structure looks good, need to add tests with the tests/ directory that compare the output of DDsmu_mocks to the output of kdcount. Let me know if you need help with that.

I will look through the kernels tomorrow. Thanks again!

nickhand commented 7 years ago

@manodeep I added tests in a similar format....take a look when you get a chance and see if anything looks out of place.

I'm also working on the corresponding theory DDsmu functions...I'll add those to this PR in the next few days.

manodeep commented 7 years ago

Looks good at a glance. I had started to implement DDsmu and DDsmu_mocks as well, and I can compare the kernels more closely. Unzip these in the ROOT/theory/ sub-directory; should be a good place to start off on. Not tested/valgrind'ed/debugged.

DDsmu_theory.zip

Thanks again!

I do have a request -- would it be possible to add a mu_max parameter to the function(s)?

nickhand commented 7 years ago

@manodeep I've added DDsmu for theory too...everything has been verified against kdcount for different mu_max values and all three instruction sets. I've also added tests for both mocks and theory.

Take a look and let me know if you need me to add/change anything.

manodeep commented 7 years ago

@nickhand Do you mind sending in the PR against the DDsmu branch (that I just created) and closing this one?

(I am happy to do so myself as well)

nickhand commented 7 years ago

@manodeep yep no problem!

manodeep commented 7 years ago

Thanks a bunch!