Separating the cluster theory and models from Firecrown into an independent module. #345

mattkwiecien · 2023-11-02T17:54:47Z

Summary

Changeset to separate the cluster theory and models logic from the internal workings of Firecrown. As an added bonus, also updated the binned cluster number counts likelihood statistic to use ModelingTools in the expected firecrown workflow.

Cluster module

Added an AbundanceData class to wrap the sacc file and to be used by the likelihood
- The sacc file data needs to be manipulated to extract the relevant cluster number counts (and masses) for the likelihood
- Put this code within this wrapper class to keep the likelihood more clear from this sacc-specific logic
Removed firecrown infrastructure code and integration code from the ClusterAbundance class.
Added a new Kernel class to represent any ingredients in the cluster likelihood
- Implemented kernels to represent the probability distribution P(z_spec|z_true), P(lambda|mass), etc
- Created a MassRichnessGaussian of Kernel, anticipating having many of these. This could potentially fall into yagni territory (you aren't gonna need it).
Added a new Integrator class for the cluster module
- Each integrator passes arguments to the cluster integrated differently, so we added this abstraction to allow specific integrators wrap and call the cluster abundance with their custom mapping
- Implemented a NumCosmo and SciPy integrator as an example.

Cluster likelihood factory function

Modified the cluster likelihood to use dependency injection for all of the choices for our likelihood. Or in other words, moved all of the specific choices (integrator, cluster kernel ingredients, data vector) to the top of the call stack, so the user can swap in and out components to the likelihood.

Other firecrown changes

Added ClusterAbundance to ModelingTools.
- As a part of this, I have ParamsMap passing through prepare.
- This may need to change now that ModelingTools is an updatable...?
Added to setup.cfg instructions to tell mypy to ignore tests. Not sure if we want to keep this.

Tests

Upped test coverage of all cluster code to 100%
Ensured previous tests and new tests are all still passing
Total runtime of all tests is < .5 seconds. All tests except two take < 0.01 seconds. Those two tests are almost regression, maybe we want to flag them as such?

Removed old cluster number count classes Added new Kernel base class, with MassObservable and Redshift subclasses Added some simple implementations of Kernel Added Kernel Factories Still WIP.

Stuck with the issue that the models in modelling tools also needs to know about the data (bounds for kernels)

Committing all work and going to start tests to check workflow.

Need to rethink how we include analytic solutions Currently this is working but not how I want it

…bserved bins.

This is going to show up as a delete and add in git.

Noticed a bug? in the previous code with limits for integration.

Updated rest of the abundance code to do vector operations Implemented a cache for HMF for now since it doesn't accept a vector of scale factors.

…bundance.

Small fixes.

Adding unit tests for the new Murata classes

…rguments, to each integrator

…pdate.

examples/cluster_number_counts/compare_integration_methods.py

firecrown/connector/cosmosis/likelihood.py

firecrown/likelihood/gauss_family/statistic/binned_cluster_number_counts.py

…uster abundance Adding some tests to capture some edge cases exposed when moving sky_area.

…f arguments on methods in tests.

Adjusting tests Adding some mypy type checking here and there

Allowing a user to pass in an integration method into the num cosmo integrator.

Updating the cluster abundance data to return SaccBins instead of confusing built in types Adding tests, and updating sacc file to reflect proper structure Fixing mypy/pylint complaints.

…g locally. Switching 3.11 syntax for older syntax. Updating cluster abundance tests to be more better.

mattkwiecien

Addition changes requested

firecrown/models/cluster/binning.py

mattkwiecien · 2023-12-04T18:32:17Z

firecrown/models/cluster/abundance_data.py

+    number count statistic.  The data in this class is specific to a single
+    survey name."""
+
+    # Hard coded in SACC, how do we want to handle this?


We probably want to enforce a standard schema for the clusters data.

firecrown/likelihood/gauss_family/statistic/binned_cluster_number_counts.py

mattkwiecien · 2023-12-04T18:50:49Z

firecrown/likelihood/gauss_family/statistic/binned_cluster_number_counts.py

+
+        self.bin_edges = sacc_adapter.get_bin_edges(
+            self.survey_name, self.cluster_properties
+        )


Assert bin dimensions are equal.

firecrown/models/cluster/kernel.py

pytest.ini

setup.cfg

tests/test_cluster_abundance.py

tests/test_cluster_integrators.py

tests/test_cluster_kernels.py

Updating verifying bin dimensions on the statistic.

Moving shared attributes to base class integrator Adding proper normalization to gaussian

Some PR review changes Adding more tests for the binning module.

mattkwiecien and others added 30 commits October 4, 2023 12:18

Temp commit

a92a0fa

Midstage of refactor.

731c567

Removed old cluster number count classes Added new Kernel base class, with MassObservable and Redshift subclasses Added some simple implementations of Kernel Added Kernel Factories Still WIP.

Another round of commits.

b6336e5

Stuck with the issue that the models in modelling tools also needs to know about the data (bounds for kernels)

flake8 is angry

6411a4f

Removing factories in favor of explicit references.

69d86c2

Added some true mass and true redshift kernels

b007840

Committing all work and going to start tests to check workflow.

Added some beginning unit tests and fixed issues involving code.

ef68364

Cosmosis running. Number counts seem correct.

97b77ee

Need to rethink how we include analytic solutions Currently this is working but not how I want it

Changing up the approach to have the cluster module compute by z, m o…

87498cf

…bserved bins.

Changes to use numcosmo integration instead.

9826c89

Moving files around to directories that make more sense.

81ccacd

This is going to show up as a delete and add in git.

Tried to tidy up the complicated integral bounds method.

a5b7ed1

Small changes to support the mean mass calculation.

442d547

Noticed a bug? in the previous code with limits for integration.

Updated numcosmo integration to pass in vectors

eb44d3f

Updated rest of the abundance code to do vector operations Implemented a cache for HMF for now since it doesn't accept a vector of scale factors.

Refactored the integration limits and integrator out of the cluster a…

23d2707

…bundance.

Switching to Bocquet16 for speed improvements

e644ad4

Small fixes.

Adding tests for cluster kernels.

c5be8c5

Added unit tests for abundance and abundance data classes.

670296a

mv __init__ to murata

807b199

rm leftover code

e95488d

update example

513b622

rename Murata classes

c82fb9d

Fixing a small bug in the new Murata class

2fa9fa7

Adding unit tests for the new Murata classes

Deferring the handling of building arguments, and how to read those a…

b2b07c3

…rguments, to each integrator

Fixing unit tests for new args reader changes

fe42495

Added covering unit tests for the integrator classes

5fd98e6

Added test for unbinned Murata

0ee47eb

Increased test coverage to 100% for all cluster classes

e820504

Improving method names, adding typing.

51ce73e

Fixing other unit tests by making params optional to modeling tools u…

96e9d0f

…pdate.

mattkwiecien marked this pull request as ready for review November 14, 2023 20:50

Merge branch 'master' into clusters-stubs

fa3dc59

mattkwiecien requested review from marcpaterno and vitenti November 14, 2023 20:52

marcpaterno reviewed Nov 20, 2023

View reviewed changes

mattkwiecien added 12 commits November 28, 2023 11:17

Moving sky area from a constructor parameter to an argument to the cl…

77b73f6

…uster abundance Adding some tests to capture some edge cases exposed when moving sky_area.

Unfortunately there's no easy way to get the mock to enforce number o…

7ebd12c

…f arguments on methods in tests.

Making cluster abundance updatable.

7070e12

Adjusting tests Adding some mypy type checking here and there

Adding a new ClusterProperty flag

6dc49dc

Updating the comparison script for numcosmo integration methods.

6676992

Allowing a user to pass in an integration method into the num cosmo integrator.

Adding new NDimensionalBin class and SaccBin implementation.

69b337b

Updating the cluster abundance data to return SaccBins instead of confusing built in types Adding tests, and updating sacc file to reflect proper structure Fixing mypy/pylint complaints.

Merge branch 'master' into clusters-stubs

734d604

Forgot to update tests for ClusterAbundance

91cf56b

Python 3.9, 3.10 tests are failing on build server, while 3.11 passin…

61c2a84

…g locally. Switching 3.11 syntax for older syntax. Updating cluster abundance tests to be more better.

Improving tests.

11e023e

Updating code to latest numcosmo version.

aa4c887

Missed places to upgrade numcosmo references.

1b51c3c

mattkwiecien commented Dec 4, 2023

View reviewed changes

mattkwiecien added 9 commits December 4, 2023 15:36

Changes to binning.py

3319fa8

Changes to test marking.

1030be4

Updating verifying bin dimensions on the statistic.

Removing numcosmo name from integrator tests.

2b31acc

Removing dimension from abundance data tests.

6cb8bff

Moving shared attributes to base class integrator Adding proper normalization to gaussian

Forgot to fix imports.

7a87d14

Making pylint happy with parenthesis

f860920

Fixing pylint CI errors.

0b7d5cf

Adding a unit test to check normalization of MurataUnbinned

bd1d08f

Some PR review changes Adding more tests for the binning module.

Pylint import order error.

5009b56

marcpaterno approved these changes Dec 11, 2023

View reviewed changes

marcpaterno merged commit 3b9a4e5 into master Dec 11, 2023
8 checks passed

marcpaterno deleted the clusters-stubs branch December 11, 2023 18:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Separating the cluster theory and models from Firecrown into an independent module. #345

Separating the cluster theory and models from Firecrown into an independent module. #345

mattkwiecien commented Nov 2, 2023 •

edited

mattkwiecien left a comment

mattkwiecien Dec 4, 2023

mattkwiecien Dec 4, 2023

mattkwiecien Dec 6, 2023

Separating the cluster theory and models from Firecrown into an independent module. #345

Separating the cluster theory and models from Firecrown into an independent module. #345

Conversation

mattkwiecien commented Nov 2, 2023 • edited

Summary

Cluster module

Cluster likelihood factory function

Other firecrown changes

Tests

mattkwiecien left a comment

Choose a reason for hiding this comment

mattkwiecien Dec 4, 2023

Choose a reason for hiding this comment

mattkwiecien Dec 4, 2023

Choose a reason for hiding this comment

mattkwiecien Dec 6, 2023

Choose a reason for hiding this comment

mattkwiecien commented Nov 2, 2023 •

edited