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: Fixes a few bugs in three point function paircounting. #516

Merged
merged 8 commits into from Aug 23, 2018

Conversation

Projects
None yet
2 participants
@rainwoodman
Member

rainwoodman commented Aug 20, 2018

This is working towards the issues pointed out by @adematti in #515.

The correctness test is expected to fail as the old result data set were probably generated from the wrong algorithm.

Two bugs were fixed:

  1. adding support to arbitrary poles input. Previously it has to be continuous starting from 0.

  2. fixing the summation of zeta. zeta is alm^2, thus shall not be summed inside callback function.

rainwoodman added some commits Aug 20, 2018

Acutally support randomly ordered ells.
The previous code requires ells to be supplied continuously.
sum alms instead of alm * alm.conj().T
alm * alm.conj().T is non-linear, and summing them directly inside
call back would have missed all cross terms.

The added pedantic test passes, but looks like the previous 3pcf
results were calculated with the wrong version of the code, and needs
to be updated for other tests to pass.

@rainwoodman rainwoodman requested a review from nickhand Aug 20, 2018

# this cannot be done in the callback because
# it is a nonlinear function (outer product) of alm.
for (l, m) in alms:
alm = alms[(l, m)]

This comment has been minimized.

@adematti

adematti Aug 21, 2018

Contributor

I believe alms should be multiplied after each primary is treated, i.e. this piece of code should be inside the loop on primaries
for iprim in range(len(pos)): (l. 112), just after l. 114
(and alms should be initialized just before l. 114)

@rainwoodman

This comment has been minimized.

Member

rainwoodman commented Aug 21, 2018

All green!

@rainwoodman rainwoodman merged commit f4623bc into bccp:master Aug 23, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.03%) to 95.105%
Details
@rainwoodman

This comment has been minimized.

Member

rainwoodman commented Aug 23, 2018

Merged for the next release. Thank you all!

@rainwoodman rainwoodman deleted the rainwoodman:threepcf-fixes branch Aug 23, 2018

@rainwoodman rainwoodman referenced this pull request Aug 23, 2018

Closed

Issues in Base3PCF #515

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