Removing cluster theory prediction from Firecrown

[eduardojsbarroso]

Description

We are removing the cluster theory module from Firecrown, as it is now an independent library (Crow).

Fixes #580

Type of change

• Refactoring
• This change requires a documentation update

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]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.0%. Comparing base (a92b250) to head (01ef4a2).

Additional details and impacted files

@@           Coverage Diff            @@
##           master     #581    +/-   ##
========================================
  Coverage   100.0%   100.0%            
========================================
  Files         151      145     -6     
  Lines        9035     8654   -381     
  Branches     1033     1030     -3     
========================================
- Hits         9035     8654   -381     

Files with missing lines	Coverage Δ
firecrown/likelihood/__init__.py	100.0% <100.0%> (ø)
firecrown/likelihood/_binned_cluster.py	100.0% <100.0%> (ø)
...ecrown/likelihood/_binned_cluster_number_counts.py	100.0% <100.0%> (ø)
.../likelihood/_binned_cluster_number_counts_shear.py	100.0% <100.0%> (ø)
firecrown/likelihood/_updatable_wrapper.py	100.0% <100.0%> (ø)
firecrown/modeling_tools/_modeling_tools.py	100.0% <ø> (ø)
firecrown/models/cluster/__init__.py	100.0% <100.0%> (ø)
firecrown/models/cluster/_abundance_data.py	100.0% <100.0%> (ø)
firecrown/models/cluster/_cluster_data.py	100.0% <100.0%> (ø)
firecrown/models/cluster/_shear_data.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.

[eduardojsbarroso]

Hi @vitenti and @marcpaterno. So I have finished what I wanted to add in this pull request. I have a couple of notes. Maybe we should have a meeting before you guys start reviewing since there are a lot of changes. I will summarize here what me and @m-aguena did and I will comment on some of my choices:

Initial goal is to not need the cluster modeling to be inside Firecrown and we use the crow package, which is under desc-crow in pipy and conda. There are several parts of the code in crow that were developed by us, you guys, and others, so let me know if you agree with these changes, if you wish to be added as maintainers of crow, or have other feedback.

• The models/cluster:

  • Everything besides the SACC data reading is now in crow.
  • We have the option to use either DeltaSigma data from SACC or Shear (which is reduced shear).
  • I updated the deltasigma_data to shear_data and now it is able to read both data types of shear. However, it gives an error if both are passed because the recipes must have different configurations for SHEAR or DELTASIGMA. So even though a SACC file can have all data types, you should not ask to use both at the same time.
  • I searched in the Firecrown repo and noted that the numcosmo integration code was being used just for clusters, so I added this to the crow repository. If you had plans or do not agree with this, I can restore it here. This choice was only so we do not need to import Firecrown in crow to use this integration.
  • The properties are also in crow for the same reason.
  • We now have a unique recipe that can be used with counts or counts+lensing. However, there is another variant which does the same thing with the same functions but does not use integration, and rather does a grid interpolation. Both options are supported in Firecrown.

• likelihood/updatable_wrapper:

  • Since now the code is external to Firecrown, we created a wrapper, updatable_wrapper, that takes all the parameters from our code and, for each object, creates an updatable that is going to be updated by the sampler. This wrapper has import and export methods, to take parameters from our code to Firecrown and vice-versa. This way there is just an Updatable class with a dictionary of parameters being updated by cosmosis, and we handle ourselves how these parameters are used by our code, the same for the cosmology parameters.
  • The names of the parameters are {crow_object}_{parameter_name}, as there are different crow_objects with the same parameter_names. I had some errors with the numcosmo sampler but this seems to work, although I know it is not the default in Firecrown, which seems to be just {parameter_name}.
  • We have one parameter in our code, cluster_concentration, that if it is None, then we use a relation to compute it, but if it is set by the sampler, we use that value. I noted I cannot pass None in the .ini parameter files, so we put in the code that negative values trigger the None function. But I would like to know if this should not be the functionality and what is considered updatable_parameters for Firecrown—should they always be something to be set and not computed if a given condition is met.
    • You will see in the tests/likelihood/gauss_family/statistic/test_binned_cluster_number_counts.py that I have to work around getting the default_parameter_values for the binned_cluster_shear parameters, because cluster_concentration = None is the default in our code, and this breaks the function.

• binned_cluster_statistics:

  • Now the binned_cluster_statistics objects use the updatable_wrapper for the updatables. Besides that, I just added the Shear and Deltasigma options. These objects work with the grid or the integral recipes. I also added a test to avoid division by 0 when the counts in a given bin are 0.
  • Renamed the binned_cluster_delta_sigma to binned_cluster_shear as it can now be used for DeltaSigma or Shear (reduced shear). Again, it should only use one or the other, but this error will already occur if one tries the read method of the class, which will call shear_data and it will point out that you cannot read both at the same time.

• Tests options to API:

  • I tried to create several examples, some using the grid and some using the exact recipe that does the integral. Once we agree on everything here, I can either provide you guys with all the options of the code to create the likelihood files, or I can help develop based on what is already done for 3x2pt, supernova, etc.
  • I also changed the generate_data file to generate both shear and delta_sigma data in a SACC file.

• Environment:

  • I added desc-crow as a package to the environment. I deleted the cache in GitHub and everything seems to be working properly.

• Documentation:

  • I still did not add documentation for the changes I made here. Maybe after we discuss everything, I can work more on this where necessary.

[vitenti]

Hello @m-aguena and @eduardojsbarroso, we are reviewing your PR and would like to propose improving the interface between Crow and Firecrown. At the moment, we cannot run static analysis because Crow does not use type hints. We also see some Crow-specific code inside Firecrown that seems unnecessary.

A cleaner approach would be to restrict the interface to configuring Crow objects using the SACC data layout, then querying Crow for the relevant parameters and passing those values during the statistical analysis. To make this workable, it would help if you could add type hints to the interfaces used by Firecrown. With that in place, we can define a small protocol that Crow can implement, allowing us to rely on static analysis without making Crow depend on Firecrown.

We can prepare a minimal example and discuss the details in a meeting. What do you think?

[eduardojsbarroso]

interfaces

Hi @vitenti, sure. If you want to prepare what you have in mind and then we can discuss during a meeting. Or even if you prefer to make a meeting first so we can align our ideas. We are available until 19/12. If we can schedule this meeting for next week, it would be great. Otherwise, after 05/01/2026 would be best. Let me know about your availability

[eduardojsbarroso]

Hi @vitenti and @marcpaterno. Do we have any news on this PR?. I note that you guys are working in some other priorities. I am updating some external code that uses the Firecrown cluster likelihood and I wanted to check before I make my changes.