From e229d35cc20686e215f2a7b077fd48d610815789 Mon Sep 17 00:00:00 2001 From: Marc Paterno Date: Fri, 8 Nov 2024 12:40:45 -0600 Subject: [PATCH] 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 | 6 +- firecrown/connector/cosmosis/likelihood.py | 90 ++++++++++--------- .../binned_cluster_number_counts.py | 6 +- firecrown/likelihood/gaussfamily.py | 4 +- firecrown/likelihood/statistic.py | 16 +--- firecrown/models/cluster/abundance_data.py | 9 +- .../cosmosis/test_cosmosis_module.py | 84 ++++++++++++++++- .../statistic/source/test_source.py | 16 +++- .../gauss_family/statistic/test_statistic.py | 17 ++++ tests/likelihood/test_gauss_family.py | 16 ++++ tests/test_base_likelihood.py | 24 +++++ tests/test_deprecated_module_imports.py | 13 +++ tests/test_descriptors.py | 7 ++ ...er_counts_magnification_bias_systematic.py | 58 ++++++++++++ tests/test_number_counts_source.py | 23 +++++ 15 files changed, 317 insertions(+), 72 deletions(-) create mode 100644 tests/likelihood/test_gauss_family.py create mode 100644 tests/test_base_likelihood.py create mode 100644 tests/test_deprecated_module_imports.py create mode 100644 tests/test_number_counts_magnification_bias_systematic.py create mode 100644 tests/test_number_counts_source.py diff --git a/examples/des_y1_3x2pt/.gitignore b/examples/des_y1_3x2pt/.gitignore index 68185264c..8d8d43fa1 100644 --- a/examples/des_y1_3x2pt/.gitignore +++ b/examples/des_y1_3x2pt/.gitignore @@ -1,3 +1,7 @@ cobaya_evaluate_output* *.png -output \ No newline at end of file +outputoutput.input.yaml +output.updated.dill_pickle +output.updated.yaml +numcosmo/ +output.input.yaml diff --git a/firecrown/connector/cosmosis/likelihood.py b/firecrown/connector/cosmosis/likelihood.py index 916f1fb8b..f6287dad5 100644 --- a/firecrown/connector/cosmosis/likelihood.py +++ b/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: diff --git a/firecrown/likelihood/binned_cluster_number_counts.py b/firecrown/likelihood/binned_cluster_number_counts.py index 8a8d0af2c..dcd2b0c53 100644 --- a/firecrown/likelihood/binned_cluster_number_counts.py +++ b/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) diff --git a/firecrown/likelihood/gaussfamily.py b/firecrown/likelihood/gaussfamily.py index 2c5e78a3c..e0e278899 100644 --- a/firecrown/likelihood/gaussfamily.py +++ b/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( diff --git a/firecrown/likelihood/statistic.py b/firecrown/likelihood/statistic.py index 16b733bae..80c279fed 100644 --- a/firecrown/likelihood/statistic.py +++ b/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. diff --git a/firecrown/models/cluster/abundance_data.py b/firecrown/models/cluster/abundance_data.py index 6baf432d0..d35883a8e 100644 --- a/firecrown/models/cluster/abundance_data.py +++ b/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 diff --git a/tests/connector/cosmosis/test_cosmosis_module.py b/tests/connector/cosmosis/test_cosmosis_module.py index 6a286db86..2dd7317c9 100644 --- a/tests/connector/cosmosis/test_cosmosis_module.py +++ b/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 + ) diff --git a/tests/likelihood/gauss_family/statistic/source/test_source.py b/tests/likelihood/gauss_family/statistic/source/test_source.py index d7e736e81..8965a7cc9 100644 --- a/tests/likelihood/gauss_family/statistic/source/test_source.py +++ b/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") diff --git a/tests/likelihood/gauss_family/statistic/test_statistic.py b/tests/likelihood/gauss_family/statistic/test_statistic.py index d1dde7756..ee24e672a 100644 --- a/tests/likelihood/gauss_family/statistic/test_statistic.py +++ b/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) diff --git a/tests/likelihood/test_gauss_family.py b/tests/likelihood/test_gauss_family.py new file mode 100644 index 000000000..0df65c66c --- /dev/null +++ b/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 ." + ), + ): + g.ConstGaussian([1]) # type: ignore diff --git a/tests/test_base_likelihood.py b/tests/test_base_likelihood.py new file mode 100644 index 000000000..51047f4ab --- /dev/null +++ b/tests/test_base_likelihood.py @@ -0,0 +1,24 @@ +""" +Tests for the module firecrown.likelihood.likelihood +""" + +import pytest +import firecrown.likelihood.likelihood as lk + + +class DefectiveLikelihood(lk.Likelihood): + """This is a defective likelhood. + + It is lacking the required `make_realization_vector` method. + """ + + def compute_loglike(self, _): + return -1.0 + + def read(self, _): + pass + + +def test_unimplemented_make_realization_vector(): + with pytest.raises(NotImplementedError): + DefectiveLikelihood().make_realization_vector() diff --git a/tests/test_deprecated_module_imports.py b/tests/test_deprecated_module_imports.py new file mode 100644 index 000000000..531d997c5 --- /dev/null +++ b/tests/test_deprecated_module_imports.py @@ -0,0 +1,13 @@ +""" +Tests for deprecated module imports. +""" + +import pytest + + +def test_import_gauss_family(): + with pytest.deprecated_call(): + # pylint: disable=import-outside-toplevel + from firecrown.likelihood.gauss_family import gauss_family + + assert gauss_family is not None diff --git a/tests/test_descriptors.py b/tests/test_descriptors.py index 7e124fc9d..101b05db9 100644 --- a/tests/test_descriptors.py +++ b/tests/test_descriptors.py @@ -216,6 +216,13 @@ def test_upper_bound_float(): assert d.x == -math.inf +def test_reading_string(): + d = HasString() + d.x = "cow" + + assert d.x == "cow" + + def test_string_conversion_failure(): d = HasString() with pytest.raises(TypeError): diff --git a/tests/test_number_counts_magnification_bias_systematic.py b/tests/test_number_counts_magnification_bias_systematic.py new file mode 100644 index 000000000..f9fb3a5e6 --- /dev/null +++ b/tests/test_number_counts_magnification_bias_systematic.py @@ -0,0 +1,58 @@ +""" +Tests for the module firecrown.likelihood.number_counts_magnification_bias_systematic. +""" + +import numpy as np +import numpy.typing as npt + +import firecrown.likelihood.number_counts as nc +import firecrown.parameters as fp +import firecrown.modeling_tools as mt + + +def test_creation_nc_magnification_bias_systematic(): + sys = nc.MagnificationBiasSystematic(sacc_tracer="lens0") + assert sys.parameter_prefix == "lens0" + assert sys.eta is None + assert sys.r_lim is None + assert sys.sig_c is None + assert sys.z_c is None + assert sys.z_m is None + + +def test_update_nc_magnification_bias_systematic( + tools_with_vanilla_cosmology: mt.ModelingTools, +): + sys = nc.MagnificationBiasSystematic(sacc_tracer="lens0") + assert sys.parameter_prefix == "lens0" + sys.update( + fp.ParamsMap( + { + "lens0_eta": 19.0, + "lens0_r_lim": 24.0, + "lens0_sig_c": 9.83, + "lens0_z_c": 0.39, + "lens0_z_m": 0.055, + } + ) + ) + assert sys.eta == 19.0 + assert sys.r_lim == 24.0 + assert sys.sig_c == 9.83 + assert sys.z_c == 0.39 + assert sys.z_m == 0.055 + + ta = nc.NumberCountsArgs( + z=np.array([1.0, 2.0]), + dndz=np.array([1.0, 0.5]), + ) + new_z: npt.NDArray[np.float64] + new_mag_bias: npt.NDArray[np.float64] + new_ta = sys.apply(tools_with_vanilla_cosmology, ta) + assert new_ta.mag_bias is not None + new_z, new_mag_bias = new_ta.mag_bias + expected_zs: npt.NDArray[np.float64] = np.array([1.0, 2.0]) + expected_mag_biases: npt.NDArray[np.float64] = np.array([0.53728088, 1.22697163]) + + assert np.allclose(new_z, expected_zs) + assert np.allclose(new_mag_bias, expected_mag_biases) diff --git a/tests/test_number_counts_source.py b/tests/test_number_counts_source.py new file mode 100644 index 000000000..0b51f9637 --- /dev/null +++ b/tests/test_number_counts_source.py @@ -0,0 +1,23 @@ +"""Tests for the NumberCountsSource base class.""" + +import firecrown.likelihood.number_counts as nc +import firecrown.metadata_types as mt +import firecrown.modeling_tools as mtools +from firecrown import parameters + + +def test_get_derived_parameters( + harmonic_bin_1: mt.InferredGalaxyZDist, + tools_with_vanilla_cosmology: mtools.ModelingTools, +): + ncs = nc.NumberCounts.create_ready(harmonic_bin_1, derived_scale=True) + ncs.update(parameters.ParamsMap({"bin_1_bias": 1.0})) + ncs.create_tracers(tools_with_vanilla_cosmology) + params = ncs.get_derived_parameters() + assert params is not None + assert len(params) == 1 + for param in params: + a, b, c = param + assert a == "TwoPoint" + assert b == "NumberCountsScale_bin_1" + assert c == 1.0