Add cluster shear profile support into SACC

LSSTDESC / firecrown

DESC Cosmology Likelihood Framework

BSD 3-Clause "New" or "Revised" License

29 stars 7 forks source link

Add cluster shear profile support into SACC #277

Closed combet closed 1 year ago

combet commented 1 year ago

This addresses issue #276.

New tracer and datatype added to sacc_support.py
New example notebook to explain usage from mock data (examples/cluster_number_counts/demo_sacc_for_clusters.ipynb).

marcpaterno commented 1 year ago

The CI system is failing this PR because it is now requiring that new code have sufficient test coverage. I will take a look at the new code and start adding tests. This might entail some refactoring of the code, if that is needed to make the code more testable.

combet commented 1 year ago

Thanks for the heads up @marcpaterno. Discussing with @eduardojsbarroso, we'd like to move this new SACC-for-cluster module into the SACC github repo, rather than having it stay in FireCrown. It will still need testing, but maybe we should only do this once in the SACC github repo? What do you think? Also pinging @vitenti.

marcpaterno commented 1 year ago

Sandro and I agree with the goal of moving what is now in firecrown/sacc_support.py, and the tests we write for it, to SACC itself. Having the tests in place should help that transition.

marcpaterno commented 1 year ago

Tests are still needed for ClusterSurveyTracer. Currently, the documentation for its to_tables method says one table is created per instance passed to to_tables. But the implementation actually returns a list with only one table; that table may have many “rows”, one for each tracer instance. Is the documentation correct, or is the implementation correct?