Skip to content
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

Bug in magnification bias #963

Merged
merged 4 commits into from
Aug 2, 2022
Merged

Bug in magnification bias #963

merged 4 commits into from
Aug 2, 2022

Conversation

damonge
Copy link
Collaborator

@damonge damonge commented Aug 1, 2022

Closes #962

@coveralls
Copy link

coveralls commented Aug 1, 2022

Pull Request Test Coverage Report for Build 2776415913

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 97.242%

Totals Coverage Status
Change from base Build 2774545550: 0.0%
Covered Lines: 4584
Relevant Lines: 4714

💛 - Coveralls

# Small bias so the magnification contribution
# is significant (if there at all)
b = np.ones_like(z)*0.1
s_no = np.ones_like(z)*0.4
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought no magnification was s=0. Am I missing something?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no magnification is s=2/5, I'm afraid :-)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, right. Might want to update the docstring here then: https://github.com/LSSTDESC/CCL/blob/master/pyccl/tracers.py#L94

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, but this is correct, right? If s is None, then it assumes s=0 (i.e. q=1) and returns the usual weak lensing kernel.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right. I was confused by the mag_bias argument in NumberCountsTracer, where mag_bias=None effectively gives q=0, and mag_bias in get_lensing_kernel, where mag_bias=None gives q=1.

src/ccl_tracers.c Show resolved Hide resolved
Copy link

@itrharrison itrharrison left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

FYI we noticed this when some tests in soliket started failing ;-)

@damonge
Copy link
Collaborator Author

damonge commented Aug 2, 2022

That's good to know. If you have an independent benchmark for magnification, we've been looking for one for a while!

@damonge damonge merged commit b47f9a4 into master Aug 2, 2022
@damonge damonge deleted the mag_fix branch August 2, 2022 10:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unexpected change in mag bias behaviour
4 participants