Skip to content

Non-limber integration support in angular c_ell calculation.#505

Merged
vitenti merged 28 commits into
masterfrom
non_limber_support
Apr 14, 2025
Merged

Non-limber integration support in angular c_ell calculation.#505
vitenti merged 28 commits into
masterfrom
non_limber_support

Conversation

@chrgeorgiou
Copy link
Copy Markdown
Member

@chrgeorgiou chrgeorgiou commented Apr 3, 2025

Description

Adds support for non-limber integration for angular power spectra in the two-point correlation.

Fixes #504

Type of change

  • New feature (non-breaking change which adds functionality)

Checklist:

The following checklist will make sure that you are following the code style and
guidelines of the project as described in the
contributing page.

  • I have run bash pre-commit-check and fixed any issues
  • I have added tests that prove my fix is effective or that my feature works
  • I have made corresponding changes to the documentation
  • I have 100% test coverage for my changes (please check this after the CI system has verified the coverage)

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 3, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.0%. Comparing base (e0f8f90) to head (c20590b).
Report is 17 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #505   +/-   ##
=======================================
  Coverage   100.0%   100.0%           
=======================================
  Files          50       50           
  Lines        4680     4775   +95     
  Branches      516      532   +16     
=======================================
+ Hits         4680     4775   +95     
Files with missing lines Coverage Δ
firecrown/connector/numcosmo/helpers.py 100.0% <100.0%> (ø)
firecrown/connector/numcosmo/numcosmo.py 100.0% <100.0%> (ø)
firecrown/likelihood/factories.py 100.0% <100.0%> (ø)
firecrown/likelihood/two_point.py 100.0% <100.0%> (ø)
firecrown/metadata_types.py 100.0% <100.0%> (ø)
...n/models/cluster/integrator/numcosmo_integrator.py 100.0% <100.0%> (ø)
firecrown/models/two_point.py 100.0% <100.0%> (ø)
firecrown/utils.py 100.0% <100.0%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@chrgeorgiou
Copy link
Copy Markdown
Member Author

@marcpaterno or @vitenti can you give me quick comments on this? This seems to be quite a small change (and important for DESC). Let me know if I am missing something I should still develop.

Comment thread firecrown/utils.py
np.array(ells),
p_of_k_a=p_of_k_a,
l_limber=l_limber,
p_of_k_a_lin=p_of_k_a_lin,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Should we always use the linear matter power spectrum (delta_matter:delta_matter) here, regardless of the tracers involved?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, I believe so. This is also how it is implemented in CCL. From the CCL documentation, this is only used with PT tracers, which need the linear matter power as input. The tracer specific calculations are encoded in the tracer objects.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There could in principle be corner cases (e.g. non-Limber for tSZ or something) where that might not be true but I wouldn't worry about this at this point. If necessary, there could be a calculate_lin_pk function for such cases but I can't think of a realistic use case now.

Comment thread firecrown/modeling_tools.py Outdated
self.cluster_abundance = cluster_abundance
self.cluster_deltasigma = cluster_deltasigma
self.ccl_factory = CCLFactory() if ccl_factory is None else ccl_factory
self.non_limber_max_ell = non_limber_max_ell
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Implementing it this way gives us a global value for non_limber_max_ell here. Is that always the case, or are there situations where non_limber_max_ell should vary depending on the tracer combination?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

While different tracer combinations might have different sensitivity to the limber approximation, I think it is safe to have this ell-limit extend to the whole two-point correlation the user requests. The method should be fast enough that this should not be a problem. If we find such a roadblock in the future, we can think of changing this but I think it's unlikely.

Copy link
Copy Markdown
Member

@arthurmloureiro arthurmloureiro Apr 3, 2025

Choose a reason for hiding this comment

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

Well... if possible, in the future, it would be useful to support turning limber to some of the tracers. While we do want non-limber for something like clustering (both spec and potentially photo) it may not be that important for the weak lensing tracers... But for now, just being able to get this option would be increadible!

Copy link
Copy Markdown
Member

@arthurmloureiro arthurmloureiro Apr 3, 2025

Choose a reason for hiding this comment

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

(But: we need to check that if what I said about WL and non-limber is true...)

@vitenti
Copy link
Copy Markdown
Collaborator

vitenti commented Apr 5, 2025

Hello @chrgeorgiou, @arthurmloureiro and @tilmantroester,

I’ve expanded the integration options to better cover the relevant CCL API. I also moved the integration control from ModelingTools to TwoPointFactory, which I believe offers more flexibility while remaining straightforward to use.

You can find the updated documentation here:
https://firecrown--505.org.readthedocs.build/en/505/_static/two_point_factories.html#controlling-integration

Let me know if you agree with this direction.

@vitenti vitenti marked this pull request as ready for review April 7, 2025 20:35
@vitenti vitenti requested a review from marcpaterno April 7, 2025 20:35
Copy link
Copy Markdown
Collaborator

@marcpaterno marcpaterno left a comment

Choose a reason for hiding this comment

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

@chrgeorgiou I've looked through this PR and it seems good to me. Since @vitenti has made some significant additions, please verify that it still satisfies your needs.

@vitenti vitenti merged commit 5ff5afd into master Apr 14, 2025
9 checks passed
@vitenti vitenti deleted the non_limber_support branch April 14, 2025 21:15
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.

Non-limber integration for two-point angular c_ells

5 participants