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

WIP: new pair counters + correlation function estimators #439

Merged
merged 47 commits into from Nov 19, 2017

Conversation

Projects
None yet
2 participants
@nickhand
Member

nickhand commented Nov 4, 2017

Addresses #405.

The goal here is to:

  • better wrapping of Corrfunc, including all of the available pair counters
  • add functionality for 1d, 2d, projected, and angular pair counters and correlation functions
  • add functionality to estimate correlation functions, either using analytic or catalog randoms
# initialize the domain
# NOTE: over-decompose by factor of 2 to trigger load balancing
grid = [
numpy.linspace(0, boxsize[0], 2*np[0] + 1, endpoint=True),

This comment has been minimized.

@rainwoodman

rainwoodman Nov 6, 2017

Member

Consider making factor of 2 a parameter.

* ``mode='2d'`` : compute pairs as a function of the 3D separation :math:`r`
and the cosine of the angle to the line-of-sight, :math:`\mu`
* ``mode='projected'`` : compute pairs as a function of distance perpendicular
and parallel to the line-of-sight, :math:`r_p` and :math:`\pi`

This comment has been minimized.

@rainwoodman

rainwoodman Nov 6, 2017

Member

Is pi a variable here?

This comment has been minimized.

@rainwoodman

rainwoodman Nov 6, 2017

Member

No it isn't. r_\pi?

This comment has been minimized.

@nickhand

nickhand Nov 6, 2017

Member

yes its r_\pi...seems like corrfunc and halotools refer to it as \pi

class MPICorrfuncCallable(object):
"""
A base class to represent MPI-enabled Corrfunc callable.

This comment has been minimized.

@rainwoodman

rainwoodman Nov 6, 2017

Member

Perhaps a link or more text what these callables are used for.

This comment has been minimized.

@nickhand

nickhand Nov 6, 2017

Member

sure no problem

This comment has been minimized.

@nickhand

nickhand Nov 6, 2017

Member

One of the things that these classes do are try to handle exceptions from Corrfunc sensibly, since it's a bit broken at the moment (as you've noted). We capture stdout/stderr and the C-level logging, and if the Corrfunc calls, print it all to screen.

There seems to be helpful C-level logging for errors in Corrfunc, but the Python exception raised is not helpful.

depending on ``mode``, the options are :math:`r`, :math:`r_p`, or
:math:`\theta`. Expected units for distances are :math:`\mathrm{Mpc}/h`
and degrees for angles. Length of nbins+1
cosmo : :class:`~nbodykit.cosmology.cosmology.Cosmology`, optional

This comment has been minimized.

@rainwoodman

rainwoodman Nov 6, 2017

Member

What if the fiducial cosmology doesn't have radiation?

This comment has been minimized.

@nickhand

nickhand Nov 6, 2017

Member

Do we assume that somewhere? It is only used to compute comoving distance to pass to Corrfunc

This comment has been minimized.

@rainwoodman

rainwoodman Nov 6, 2017

Member

Class can't do without radiation..

This comment has been minimized.

@nickhand

nickhand Nov 6, 2017

Member

oh sure, but that's a more general issue with the Cosmology class. We should raise a more helpful message when Tcmb0 is 0 in Cosmology?

@rainwoodman

This comment has been minimized.

Member

rainwoodman commented Nov 6, 2017

There is no longer 'xi' any more?

@nickhand

This comment has been minimized.

Member

nickhand commented Nov 6, 2017

I've just added a first pass at the pair counting based 2PCF classes. These classes roughly follow the following logic for xi(r), xi(r,mu), xi(rp, pi), and xi(theta)

  1. check if simulation box with PBC turned on; if no randoms catalog explicitly supplied, use analytic randoms and do DD/RR-1
  2. otherwise, require a randoms catalog, and do Landy-Szalay estimator to get CF and pair counts
@nickhand

This comment has been minimized.

Member

nickhand commented Nov 18, 2017

I think this is also ready to merge on my end now, if you want to take a look @rainwoodman

nickhand added some commits Nov 19, 2017

@nickhand

This comment has been minimized.

Member

nickhand commented Nov 19, 2017

@rainwoodman I'm going to merge this because I need it for benchmarks, but we can add updates later if you have comments

@nickhand nickhand merged commit 211419b into bccp:master Nov 19, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.3%) to 95.49%
Details

@nickhand nickhand deleted the nickhand:correlation branch Nov 19, 2017

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