More test coverage (#470)

* Ignore new generated files
* Replace untested validation with assertion
* Add testing of GaussFamily initialization failure mode
* Add test for reading of TypeString descriptor
* Add testing for GuardedStatistic.read
* Remove laundering of exception thrown by statistic
* Add test of module's execute function
* Add test of reading parameters in NumberCountsSource
* Refactor to allow testing of form_error_message
* Extract calculate_firecrown_params for testing

examples/des_y1_3x2pt/.gitignore

@@ -1,3 +1,7 @@
 cobaya_evaluate_output*
 *.png
-output
+outputoutput.input.yaml
+output.updated.dill_pickle
+output.updated.yaml
+numcosmo/
+output.input.yaml

firecrown/connector/cosmosis/likelihood.py

@@ -116,7 +116,9 @@ def execute(self, sample: cosmosis.datablock) -> int:
         # the likelihood module. And it requires updates to Firecrown to split the
         # calculations. e.g., data_vector/firecrown_theory  data_vector/firecrown_data
 
-        firecrown_params = self.calculate_firecrown_params(sample)
+        firecrown_params = calculate_firecrown_params(
+            self.sampling_sections, self.firecrown_module_name, sample
+        )
         firecrown_params = ParamsMap(firecrown_params | self.map.asdict())
         firecrown_params.use_lower_case_keys(True)
         self.update_likelihood_and_tools(firecrown_params)
@@ -228,54 +230,60 @@ def update_likelihood_and_tools(self, firecrown_params: ParamsMap) -> None:
             self.likelihood.update(firecrown_params)
             self.tools.update(firecrown_params)
         except MissingSamplerParameterError as exc:
-            msg = self.form_error_message(exc)
+            msg = form_error_message(self.sampling_sections, exc)
             raise RuntimeError(msg) from exc
 
-    def form_error_message(self, exc: MissingSamplerParameterError) -> str:
-        """Form the error message that will be used to report a missing parameter.
 
-        This error message will also include when that parameter should have been
-        supplied by the sampler.
+def calculate_firecrown_params(
+    sampling_sections: list[str], firecrown_module_name: str, sample: cosmosis.datablock
+) -> ParamsMap:
+    """Calculate the ParamsMap for this sample.
 
-        :param exc: the missing parameter error
-        :return: the error message
-        """
-        msg = (
-            "A required parameter was not found in any of the "
-            "sections searched on DataBlock.\n"
-            "These are specified by the space-separated string "
-            "`sampling_parameter_sections`.\n"
-            "The supplied value was"
-        )
-        sampling_parameters_sections = " ".join(self.sampling_sections)
-        if sampling_parameters_sections:
-            msg += f": `{sampling_parameters_sections}`"
-        else:
-            msg += " an empty string."
-        msg += f"\nThe missing parameter is named: `{exc.parameter}`\n"
-        return msg
+    :param sample: the sample generated by the sampler
+    :return: a ParamsMap with the firecrown parameters
+    """
+    firecrown_params = ParamsMap()
+    for section in sampling_sections:
+        section_params = extract_section(sample, section)
+        shared_keys = section_params.to_set().intersection(firecrown_params)
+        if len(shared_keys) > 0:
+            raise RuntimeError(
+                f"The following keys `{shared_keys}` appear "
+                f"in more than one section used by the "
+                f"module {firecrown_module_name}."
+            )
 
-    def calculate_firecrown_params(self, sample: cosmosis.datablock) -> ParamsMap:
-        """Calculate the ParamsMap for this sample.
+        firecrown_params = ParamsMap({**firecrown_params, **section_params.data})
 
-        :param sample: the sample generated by the sampler
-        :return: a ParamsMap with the firecrown parameters
-        """
-        firecrown_params = ParamsMap()
-        for section in self.sampling_sections:
-            section_params = extract_section(sample, section)
-            shared_keys = section_params.to_set().intersection(firecrown_params)
-            if len(shared_keys) > 0:
-                raise RuntimeError(
-                    f"The following keys `{shared_keys}' appear "
-                    f"in more than one section used by the "
-                    f"module {self.firecrown_module_name}."
-                )
+    firecrown_params.use_lower_case_keys(True)
+    return firecrown_params
 
-            firecrown_params = ParamsMap({**firecrown_params, **section_params.data})
 
-        firecrown_params.use_lower_case_keys(True)
-        return firecrown_params
+def form_error_message(
+    sampling_sections: list[str], exc: MissingSamplerParameterError
+) -> str:
+    """Form the error message that will be used to report a missing parameter.
+
+    This error message will also include when that parameter should have been
+    supplied by the sampler.
+
+    :param exc: the missing parameter error
+    :return: the error message
+    """
+    msg = (
+        "A required parameter was not found in any of the "
+        "sections searched on DataBlock.\n"
+        "These are specified by the space-separated string "
+        "`sampling_parameter_sections`.\n"
+        "The supplied value was"
+    )
+    sampling_parameters_sections = " ".join(sampling_sections)
+    if sampling_parameters_sections:
+        msg += f": `{sampling_parameters_sections}`"
+    else:
+        msg += " an empty string."
+    msg += f"\nThe missing parameter is named: `{exc.parameter}`\n"
+    return msg
 
 
 def setup(config: cosmosis.datablock) -> FirecrownLikelihood:

firecrown/likelihood/binned_cluster_number_counts.py

@@ -69,11 +69,7 @@ def read(self, sacc_data: sacc.Sacc) -> None:
             self.survey_name, self.cluster_properties
         )
         for bin_edge in self.bins:
-            if bin_edge.dimension != self.bins[0].dimension:
-                raise ValueError(
-                    "The cluster number counts statistic requires all bins to be the "
-                    "same dimension."
-                )
+            assert bin_edge.dimension == self.bins[0].dimension
 
         super().read(sacc_data)
 

firecrown/likelihood/gaussfamily.py

@@ -159,8 +159,8 @@ def __init__(
         for i, s in enumerate(statistics):
             if not isinstance(s, Statistic):
                 raise ValueError(
-                    f"statistics[{i}] is not an instance of Statistic: {s}"
-                    f"it is a {type(s)} instead."
+                    f"statistics[{i}] is not an instance of Statistic."
+                    f" It is a {type(s)}."
                 )
 
         self.statistics: UpdatableCollection[GuardedStatistic] = UpdatableCollection(

firecrown/likelihood/statistic.py

@@ -82,12 +82,8 @@ def read(self, _: sacc.Sacc) -> None:
 
         :param _: currently unused, but required by the interface.
         """
+        assert len(self.get_data_vector()) > 0
         self.ready = True
-        if len(self.get_data_vector()) == 0:
-            raise RuntimeError(
-                f"the statistic {self} has read a data vector "
-                f"of length 0; the length must be positive"
-            )
 
     def _reset(self):
         """Reset this statistic.
@@ -176,15 +172,7 @@ def read(self, sacc_data: sacc.Sacc) -> None:
         """
         if self.statistic.ready:
             raise RuntimeError("Firecrown has called read twice on a GuardedStatistic")
-        try:
-            self.statistic.read(sacc_data)
-        except TypeError as exc:
-            msg = (
-                f"A statistic of type {type(self.statistic).__name__} has raised "
-                f"an exception during `read`.\nThe problem may be a malformed "
-                f"SACC data object."
-            )
-            raise RuntimeError(msg) from exc
+        self.statistic.read(sacc_data)
 
     def get_data_vector(self) -> DataVector:
         """Return the contained :class:`Statistic`'s data vector.

firecrown/models/cluster/abundance_data.py

@@ -90,12 +90,9 @@ def _all_bin_combinations_for_data_type(self, data_type: str) -> npt.NDArray:
                 f"{data_type} data type."
             )
 
-        if bins_combos_for_type.shape[1] != 3:
-            raise ValueError(
-                "The SACC file must contain 3 tracers for the "
-                "cluster_counts data type: cluster_survey, "
-                "redshift argument and mass argument tracers."
-            )
+        assert (
+            bins_combos_for_type.shape[1] == 3
+        ), "The SACC file must contain 3 tracers for the cluster_counts data type."
 
         return bins_combos_for_type
 

tests/connector/cosmosis/test_cosmosis_module.py

@@ -14,8 +14,11 @@
 from firecrown.likelihood.likelihood import NamedParameters
 from firecrown.connector.cosmosis.likelihood import (
     FirecrownLikelihood,
-    extract_section,
     MissingSamplerParameterError,
+    calculate_firecrown_params,
+    execute,
+    extract_section,
+    form_error_message,
 )
 
 
@@ -175,7 +178,12 @@ def fixture_firecrown_mod_with_two_point_harmonic(
 
 @pytest.fixture(name="sample_with_cosmo")
 def fixture_sample_with_cosmo() -> DataBlock:
-    """Return a DataBlock that contains some cosmological parameters."""
+    """Return a DataBlock that contains some cosmological parameters.
+
+    It will have one section, cosmological_parameters.
+    This section will have parameters h0, omega_b, omega_c, omega_k, omega_nu, w,
+    and wa.
+    """
     result = DataBlock()
     params = {
         "h0": 0.83,
@@ -193,11 +201,18 @@ def fixture_sample_with_cosmo() -> DataBlock:
 
 @pytest.fixture(name="minimal_sample")
 def fixture_minimal_sample(sample_with_cosmo: DataBlock) -> DataBlock:
+    """Return a DataBlock containing two sections: cosmological_parameters and distances
+
+    See sample_with_cosmo for the parameters in the cosmological_parameters section.
+    The section distances will have parameters d_m, h, and z.
+    All will be double arrays of length 100.
+    """
     with open("tests/distances.yml", encoding="utf-8") as stream:
         rawdata = yaml.load(stream, yaml.CLoader)
     sample = sample_with_cosmo
     for section_name, section_data in rawdata.items():
         for parameter_name, value in section_data.items():
+            assert len(value) == 100
             sample.put(section_name, parameter_name, np.array(value))
     return sample
 
@@ -351,6 +366,14 @@ def test_module_exec_working(
     firecrown_mod_with_const_gaussian: FirecrownLikelihood, sample_with_M: DataBlock
 ):
     assert firecrown_mod_with_const_gaussian.execute(sample_with_M) == 0
+    assert sample_with_M.get_double("likelihoods", "firecrown_like") < 0.0
+
+
+def test_execute_function(
+    firecrown_mod_with_const_gaussian: FirecrownLikelihood, sample_with_M: DataBlock
+):
+    assert execute(sample_with_M, firecrown_mod_with_const_gaussian) == 0
+    assert sample_with_M.get_double("likelihoods", "firecrown_like") < 0.0
 
 
 def test_module_exec_with_two_point_real(
@@ -564,3 +587,60 @@ def test_mapping_cosmosis_pk_nonlin(mapping_cosmosis):
             mapping_cosmosis.transform_p_k_h3_to_p_k(matter_power_nl_p_k)
         ),
     )
+
+
+def test_form_error_message_with_sampling_sections():
+    sampling_sections = ["section1", "section2"]
+    exc = MissingSamplerParameterError("missing_param")
+    expected_msg = (
+        "A required parameter was not found in any of the sections searched on DataBlock.\n"  # noqa
+        "These are specified by the space-separated string `sampling_parameter_sections`.\n"  # noqa
+        "The supplied value was: `section1 section2`\n"
+        "The missing parameter is named: `missing_param`\n"
+    )
+    assert form_error_message(sampling_sections, exc) == expected_msg
+
+
+def test_form_error_message_with_empty_sampling_sections():
+    sampling_sections: list[str] = []
+    exc = MissingSamplerParameterError("missing_param")
+    #  flake8: noqa
+    expected_msg = (
+        "A required parameter was not found in any of the sections searched on DataBlock.\n"  # noqa
+        "These are specified by the space-separated string `sampling_parameter_sections`.\n"  # noqa
+        "The supplied value was an empty string.\n"
+        "The missing parameter is named: `missing_param`\n"
+    )
+    assert form_error_message(sampling_sections, exc) == expected_msg
+
+
+def test_form_error_message_with_single_sampling_section():
+    sampling_sections = ["section1"]
+    exc = MissingSamplerParameterError("missing_param")
+    #  flake8: noqa
+    expected_msg = (
+        "A required parameter was not found in any of the sections searched on DataBlock.\n"  # noqa
+        "These are specified by the space-separated string `sampling_parameter_sections`.\n"  # noqa
+        "The supplied value was: `section1`\n"
+        "The missing parameter is named: `missing_param`\n"
+    )
+    assert form_error_message(sampling_sections, exc) == expected_msg
+
+
+def test_form_error_message_with_missing_sampler_parameter_error():
+    sampling_sections = ["section1", "section2"]
+    exc = MissingSamplerParameterError("missing_param")
+    assert "missing_param" in form_error_message(sampling_sections, exc)
+
+
+def test_same_param_names_in_different_sections_failure(sample_with_M: DataBlock):
+    sample_with_M.put_double("section1", "a", 1.0)
+    sample_with_M.put_double("section2", "a", 10.0)
+    with pytest.raises(
+        RuntimeError,
+        match="The following keys `{'a'}` appear in more than one section used by"
+        " the module firecrown_mod.",
+    ):
+        _ = calculate_firecrown_params(
+            ["section1", "section2"], "firecrown_mod", sample_with_M
+        )

tests/likelihood/gauss_family/statistic/source/test_source.py

@@ -12,6 +12,7 @@
 import pyccl
 import sacc
 
+from firecrown.likelihood.number_counts import NumberCountsArgs
 from firecrown.modeling_tools import ModelingTools
 from firecrown.likelihood.source import (
     SourceGalaxy,
@@ -106,7 +107,7 @@ def test_tracer_construction_with_name(empty_pyccl_tracer):
     assert not named.has_pt
 
 
-def test_linear_bias_systematic():
+def test_nc_linear_bias_systematic():
     a = nc.LinearBiasSystematic("xxx")
     assert isinstance(a, nc.LinearBiasSystematic)
     assert a.parameter_prefix == "xxx"
@@ -129,6 +130,19 @@ def test_linear_bias_systematic():
     assert a.z_piv is None
 
 
+def test_nc_nonlinear_bias_systematic_tracer_args_missing(
+    tools_with_vanilla_cosmology: ModelingTools,
+):
+    a = nc.LinearBiasSystematic("xxx")
+    # The values in the ParamsMap and the tracer_args are set to allow easy verification
+    # that a tracer_args of None is handled correctly.
+    a.update(ParamsMap({"xxx_alphag": 1.0, "xxx_alphaz": 1.0, "xxx_z_piv": 0.0}))
+    tracer_args = NumberCountsArgs(z=np.array([0.0]), dndz=np.array([1.0]))
+    new_tracer_args = a.apply(tools_with_vanilla_cosmology, tracer_args)
+    assert new_tracer_args.bias is not None
+    assert np.allclose(new_tracer_args.bias, np.array([1.0]))
+
+
 def test_trivial_source_galaxy_construction():
     trivial = TrivialSourceGalaxy(sacc_tracer="no-sacc-tracer")
 

tests/likelihood/gauss_family/statistic/test_statistic.py

@@ -99,6 +99,14 @@ def test_guarded_statistic_read_only_once(
         gs.read(sacc_data_for_trivial_stat)
 
 
+def test_guarded_static_read_data_vector(
+    sacc_data_for_trivial_stat: sacc.Sacc, trivial_stats: list[stat.TrivialStatistic]
+):
+    gs = stat.GuardedStatistic(trivial_stats.pop())
+    gs.read(sacc_data_for_trivial_stat)
+    assert_allclose(gs.get_data_vector(), [1.0, 4.0, -3.0])
+
+
 def test_guarded_statistic_get_data_before_read(trivial_stats):
     s = trivial_stats.pop()
     with pytest.raises(
@@ -116,6 +124,15 @@ def test_statistic_get_data_vector_before_read():
         _ = s.get_data_vector()
 
 
+def test_guarded_statiistic_compute_theory_before_read():
+    gs = stat.GuardedStatistic(stat.TrivialStatistic())
+    with pytest.raises(
+        RuntimeError,
+        match="The statistic .* was used for calculation before `read` was called",
+    ):
+        gs.compute_theory_vector(ModelingTools())
+
+
 def test_statistic_get_data_vector_after_read(sacc_data_for_trivial_stat):
     s = stat.TrivialStatistic()
     s.read(sacc_data_for_trivial_stat)

tests/likelihood/test_gauss_family.py

@@ -0,0 +1,16 @@
+"""Tests for GaussFamily base class."""
+
+import re
+
+import pytest
+import firecrown.likelihood.gaussian as g
+
+
+def test_init_rejects_non_statistics():
+    with pytest.raises(
+        ValueError,
+        match=re.escape(
+            "statistics[0] is not an instance of Statistic. It is a <class 'int'>."
+        ),
+    ):
+        g.ConstGaussian([1])  # type: ignore