New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

adding w_p jackknife mock observable #814

Merged
merged 14 commits into from Oct 17, 2017

Conversation

Projects
None yet
2 participants
@duncandc
Copy link
Contributor

duncandc commented Oct 11, 2017

This pull request adds a projected two-point correlation function that calculates jackknife covariance matrices to the mock_observables.two_point_clustering package to address the long-standing issue #234.

Adding this function required coding up a redshift space jackknife pair counter and the associated Cython engine.

aphearin and others added some commits Sep 27, 2017

Current status of branch developments:
The following files have been modified relative to master:

halotools/mock_observables/pair_counters/__init__.py
halotools/mock_observables/pair_counters/cpairs/__init__.py
halotools/mock_observables/pair_counters/cpairs/npairs_jackknife_rp_pi_engine.pyx
halotools/mock_observables/pair_counters/cpairs/setup_package.py
halotools/mock_observables/pair_counters/npairs_jackknife_rp_pi.py
halotools/mock_observables/two_point_clustering/__init__.py
halotools/mock_observables/two_point_clustering/wp_jackknife.py

In particular, the user-facing function wp_jackknife has all the backend machinery in place so that the cython compiles and is appropriately linked, and all the relevant __init__ modules have the appropriate modifications.

There are two failing doctests, one for wp_jackknife and another for npairs_jackknife_rp_pi. The failures are due to a NotImplementedError placed at the point where I left off. Running the test suite for either module will give an error message explaining that the primary remaining task is writing the npairs_jackknife_rp_pi_engine kernel to do 2+1 counting. Once that is implemented, the function *should* be correct, and the only remaining tasks are double-checking the docstrings of wp_jackknife and npairs_jackknife_rp_pi, writing a unit-test or two, and updating the changelog.

@duncandc - I hoped this would help alleviate some of the infrastructure burden, but let me know if you have any questions about finishing up.

@aphearin aphearin added this to the v0.6 milestone Oct 11, 2017

@aphearin

This comment has been minimized.

Copy link
Contributor

aphearin commented Oct 17, 2017

@duncandc - is this ready to merge as is? Or were you postponing until you can figure out a good unit-test?

@duncandc

This comment has been minimized.

Copy link
Contributor Author

duncandc commented Oct 17, 2017

I think this can merge. I was postponing until we could figure out the jackknife weighting scheme, but let's leave that to a dedicated pull request for both wp and xi if need be.

@aphearin aphearin merged commit f66a9cd into astropy:master Oct 17, 2017

2 of 3 checks passed

coverage/coveralls Coverage decreased (-0.02%) to 87.62%
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@duncandc duncandc deleted the duncandc:wp_jackknife branch Oct 27, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment