diff --git a/changelog.d/20260723_143802_jennifer.pollack_227_new_feature_add_quality_control_configuration_interface.md b/changelog.d/20260723_143802_jennifer.pollack_227_new_feature_add_quality_control_configuration_interface.md index 8e1875ea..7d861dce 100644 --- a/changelog.d/20260723_143802_jennifer.pollack_227_new_feature_add_quality_control_configuration_interface.md +++ b/changelog.d/20260723_143802_jennifer.pollack_227_new_feature_add_quality_control_configuration_interface.md @@ -35,4 +35,8 @@ For top level release notes, leave all the headers commented out. configuration framework, including typed configuration objects, YAML parsing, and validation for quality metrics, rejection policies, and reporting. +- Extended the quality control configuration framework with configurable + resource declarations and metric resource requirements, including + validation of resource references and consistency between quality metrics + and rejection policies. diff --git a/config/quality_control.yaml b/config/quality_control.yaml index 3f45b313..501543a4 100644 --- a/config/quality_control.yaml +++ b/config/quality_control.yaml @@ -1,12 +1,28 @@ +resources: + + psf_models: + standard: + inference_config: inference_standard.yaml + oversampled: + inference_config: inference_oversampled.yaml + metrics: mask_obscuration: enabled: true + params: + aperture: gaussian + sigma: 2.5 goodness_of_fit: enabled: true + required_resources: + - psf_models.standard + params: - inference_config: inference_config.yaml + statistic: reduced_chi_square + normalize_residuals: true + rejection: @@ -21,4 +37,4 @@ rejection: reporting: save_metrics: true - log_statistics: true \ No newline at end of file + log_statistics: true \ No newline at end of file diff --git a/src/wf_psf/quality_control/config.py b/src/wf_psf/quality_control/config.py index ce240563..b9e9e074 100644 --- a/src/wf_psf/quality_control/config.py +++ b/src/wf_psf/quality_control/config.py @@ -9,8 +9,11 @@ """ from __future__ import annotations +from collections.abc import Mapping from dataclasses import dataclass, field from pathlib import Path +from typing import Any + from wf_psf.utils.read_config import read_yaml @@ -20,12 +23,19 @@ class QualityMetricConfig: Attributes ---------- - enabled: Whether the quality metric is enabled. - params: Arbitrary quality metric-specific parameters. + enabled : bool + Whether the quality metric is enabled. + + params : dict + Quality metric-specific parameters. + + required_resources : list[str] + Identifiers of resources required to compute the quality metric. """ enabled: bool = True params: dict = field(default_factory=dict) + required_resources: list[str] = field(default_factory=list) @dataclass @@ -34,8 +44,11 @@ class RejectionPolicyConfig: Attributes ---------- - enabled: Whether rejection policy is enabled. - threshold: Numeric threshold used to trigger rejection policy, or None. + enabled : bool + Whether rejection policy is enabled. + + threshold : float | None + Numeric threshold used by the rejection policy, or None if not applicable. """ enabled: bool = False @@ -48,23 +61,47 @@ class ReportingConfig: Attributes ---------- - save_metrics: Persist computed metrics to storage. - log_statistics: Emit statistics to the logging system. + save_metrics : bool + Persist computed metrics to storage. + + log_statistics : bool + Emit statistics to the logging system. """ save_metrics: bool = False log_statistics: bool = False +@dataclass +class ResourcesConfig: + """Configuration for resources available for quality control execution. + + Attributes + ---------- + available : dict + Mapping of resource types to configured resource identifiers and parameters. + """ + + available: dict[str, Any] = field(default_factory=dict) + + @dataclass class QualityControlConfig: """Top-level quality control configuration. Attributes ---------- - metrics: Mapping of metric name -> QualityMetricConfig. - rejection: Mapping of check name -> RejectionPolicyConfig. - reporting: ReportingConfig instance. + metrics : dict + Mapping of metric name to QualityMetricConfig instances. + + rejection : dict + Mapping of rejection policy name to RejectionPolicyConfig instances. + + reporting : ReportingConfig + ReportingConfig instance. + + resources : ResourcesConfig + Available resources used during quality control execution. """ metrics: dict[str, QualityMetricConfig] = field(default_factory=dict) @@ -73,22 +110,44 @@ class QualityControlConfig: reporting: ReportingConfig = field(default_factory=ReportingConfig) + resources: ResourcesConfig = field(default_factory=ResourcesConfig) + # config section parsers def parse_metrics_config( - section: dict[str, dict] | None, + section: Mapping[str, Any] | None, ) -> dict[str, QualityMetricConfig]: - """Parse quality metric configuration section.""" + """Parse the quality metrics configuration section. + + Parameters + ---------- + section : Mapping[str, Any] or None + Raw quality metric configuration. If None, an empty configuration + is returned. + + Returns + ------- + dict[str, QualityMetricConfig] + Parsed quality metric configurations keyed by metric name. + + Raises + ------ + TypeError + If the configuration section or any metric configuration is not a + mapping, if ``enabled`` is not boolean, if ``params`` is not a + mapping, or if required resource identifiers is not a list, or if + its entries are not strings. + """ if section is None: return {} - if not isinstance(section, dict): - raise TypeError("Metrics configuration must be a mapping") + if not isinstance(section, Mapping): + raise TypeError("Metrics configuration must be a mapping.") metrics = {} for name, cfg in section.items(): - if not isinstance(cfg, dict): + if not isinstance(cfg, Mapping): raise TypeError(f"Metric configuration '{name}' must be a mapping.") enabled = cfg.get("enabled", True) @@ -98,31 +157,58 @@ def parse_metrics_config( params = cfg.get("params", {}) - if not isinstance(params, dict): + if not isinstance(params, Mapping): raise TypeError(f"Metric parameters for '{name}' must be a mapping.") + required_resources = cfg.get("required_resources", []) + + if not isinstance(required_resources, list): + raise TypeError(f"Required resources for metric '{name}' must be a list.") + + if not all(isinstance(resource, str) for resource in required_resources): + raise TypeError( + f"Required resources for metric '{name}' must contain only strings." + ) + metrics[name] = QualityMetricConfig( - enabled=enabled, - params=params, + enabled=enabled, params=dict(params), required_resources=required_resources ) return metrics def parse_rejection_policy_config( - config: dict[str, dict] | None, + section: Mapping[str, Any] | None, ) -> dict[str, RejectionPolicyConfig]: - """Parse rejection policy configuration section.""" - if config is None: + """Parse the rejection policy configuration section. + + Parameters + ---------- + section : Mapping[str, Any] or None + Raw rejection policy configuration. If None, an empty configuration + is returned. + + Returns + ------- + dict[str, RejectionPolicyConfig] + Parsed rejection policy configurations keyed by metric name. + + Raises + ------ + TypeError + If the configuration section or a policy configuration is not a + mapping. + """ + if section is None: return {} - if not isinstance(config, dict): + if not isinstance(section, Mapping): raise TypeError("Rejection policy configuration must be a mapping.") policies = {} - for name, cfg in config.items(): - if not isinstance(cfg, dict): + for name, cfg in section.items(): + if not isinstance(cfg, Mapping): raise TypeError( f"Rejection policy configuration '{name}' must be a mapping." ) @@ -132,21 +218,151 @@ def parse_rejection_policy_config( return policies -def parse_reporting_config(config: dict | None) -> ReportingConfig: - """Parse reporting configuration section.""" - if config is None: +def parse_reporting_config(section: Mapping[str, Any] | None) -> ReportingConfig: + """Parse the reporting configuration section. + + Parameters + ---------- + section : dict or None + Raw reporting configuration. If None, the default reporting + configuration is returned. + + Returns + ------- + ReportingConfig + Parsed reporting configuration. + + Raises + ------ + TypeError + If the configuration section is not a mapping. + """ + if section is None: return ReportingConfig() - if not isinstance(config, dict): + if not isinstance(section, Mapping): raise TypeError("Reporting configuration must be a mapping.") - return ReportingConfig(**config) + return ReportingConfig(**section) + + +def parse_resources_config( + config: Mapping[str, Any] | None, +) -> ResourcesConfig: + """Parse the resources configuration section. + + Parameters + ---------- + config : Mapping or None + Raw resource configuration. If None, an empty resource + configuration is returned. + + Returns + ------- + ResourcesConfig + Parsed resource configuration. + + Raises + ------ + TypeError + If the configuration section is not a mapping. + """ + if config is None: + return ResourcesConfig() + + if not isinstance(config, Mapping): + raise TypeError("Resources configuration must be a mapping.") + + return ResourcesConfig(available=dict(config)) + + +def validate_quality_control_config(config: QualityControlConfig) -> None: + """Validate internal consistency of a quality control configuration. + + Parameters + ---------- + config : QualityControlConfig + Parsed quality control configuration. + + Raises + ------ + ValueError + If any cross-section configuration dependency is invalid. + """ + validate_metric_resources(config) + validate_rejection_policy_metrics(config) + + +def validate_metric_resources(config: QualityControlConfig) -> None: + """Validate that all metric resource requirements can be resolved. + + Parameters + ---------- + config : QualityControlConfig + Parsed quality control configuration. + + Raises + ------ + ValueError + If a required resource identifier is malformed or references an + unavailable resource. + """ + for metric_name, metric in config.metrics.items(): + for req in metric.required_resources: + parts = req.split(".") + + if len(parts) != 2 or not all(parts): + raise ValueError( + f"Resource identifier '{req}' must have the form " + "'.'." + ) + + resource_type, resource_name = parts + resources = config.resources.available + + if ( + resource_type not in resources + or resource_name not in resources[resource_type] + ): + raise ValueError( + f"Metric '{metric_name}' requires unknown resource '{req}'." + ) + + +def validate_rejection_policy_metrics(config: QualityControlConfig) -> None: + """Validate rejection policies against configured quality metrics. + + Parameters + ---------- + config : QualityControlConfig + Parsed quality control configuration. + + Raises + ------ + ValueError + If an enabled rejection policy references an unknown or disabled + quality metric. + """ + for metric_name, metric_rejection_policy in config.rejection.items(): + if not metric_rejection_policy.enabled: + continue + + if metric_name not in config.metrics: + raise ValueError( + f"Rejection policy configured for unknown metric '{metric_name}' " + ) + + if not config.metrics[metric_name].enabled: + raise ValueError( + f"Rejection policy cannot be enabled because metric '{metric_name}' is disabled." + ) SECTION_PARSERS = { "metrics": parse_metrics_config, "rejection": parse_rejection_policy_config, "reporting": parse_reporting_config, + "resources": parse_resources_config, } @@ -168,7 +384,22 @@ def __init__(self, qc_config_path: str | Path): self.qc_config_path = qc_config_path def load(self) -> QualityControlConfig: - """Load and parse configuration file.""" + """Load, parse, and validate the quality control configuration. + + Returns + ------- + QualityControlConfig + Parsed and validated quality control configuration. + + Raises + ------ + TypeError + If a configuration section has an invalid structure or type. + + ValueError + If the parsed configuration contains inconsistent references + between metrics, resources, or rejection policies. + """ qc_config = read_yaml(self.qc_config_path) config = {} @@ -176,4 +407,8 @@ def load(self) -> QualityControlConfig: values = qc_config.get(section, {}) config[section] = parser(values) - return QualityControlConfig(**config) + qc = QualityControlConfig(**config) + + validate_quality_control_config(qc) + + return qc diff --git a/src/wf_psf/tests/test_quality_control/config_test.py b/src/wf_psf/tests/test_quality_control/config_test.py index 25b427eb..e06668c8 100644 --- a/src/wf_psf/tests/test_quality_control/config_test.py +++ b/src/wf_psf/tests/test_quality_control/config_test.py @@ -6,6 +6,7 @@ """ +from contextlib import nullcontext as does_not_raise from pathlib import Path import pytest from wf_psf.quality_control.config import ( @@ -14,6 +15,12 @@ QualityMetricConfig, RejectionPolicyConfig, ReportingConfig, + ResourcesConfig, +) +from wf_psf.quality_control.config import ( + parse_resources_config, + validate_metric_resources, + validate_rejection_policy_metrics, ) @@ -22,16 +29,73 @@ def load_config(config_file: str) -> QualityControlConfig: return handler.load() +@pytest.fixture +def qc_config_factory(): + def factory( + *, + required_resources=None, + rejection_metric=None, + resources=None, + metrics=None, + rejection=None, + ): + metric_default = { + "goodness_of_fit": QualityMetricConfig( + enabled=True, + required_resources=required_resources or [], + ) + } + + resources_default = ResourcesConfig( + available={ + "psf_models": { + "standard": { + "inference_config": "inference_standard.yaml", + } + } + } + ) + + rejection_default = { + rejection_metric or "goodness_of_fit": RejectionPolicyConfig( + enabled=True, + threshold=0.25, + ) + } + + return QualityControlConfig( + metrics=metric_default if metrics is None else metrics, + resources=resources_default if resources is None else resources, + rejection=rejection_default if rejection is None else rejection, + ) + + return factory + + +# Test for config loading and parsers def test_quality_control_config_loading(): config = load_config("valid/quality_control.yaml") + assert isinstance(config.resources, ResourcesConfig) + assert "standard" in config.resources.available["psf_models"] + assert "oversampled" in config.resources.available["psf_models"] + assert ( + config.resources.available["psf_models"]["standard"]["inference_config"] + == "inference_standard.yaml" + ) + assert ( + config.resources.available["psf_models"]["oversampled"]["inference_config"] + == "inference_oversampled.yaml" + ) + assert "mask_obscuration" in config.metrics assert isinstance(config.metrics["mask_obscuration"], QualityMetricConfig) assert config.metrics["mask_obscuration"].enabled is True + assert config.metrics["mask_obscuration"].required_resources == [] assert isinstance(config.metrics["goodness_of_fit"], QualityMetricConfig) - assert config.metrics["goodness_of_fit"].params["inference_config"] == ( - "inference_config.yaml" + assert config.metrics["goodness_of_fit"].required_resources == ( + ["psf_models.standard"] ) assert isinstance(config.rejection["mask_obscuration"], RejectionPolicyConfig) @@ -41,6 +105,34 @@ def test_quality_control_config_loading(): assert config.reporting.save_metrics is True +def test_parse_resources_config(): + config = { + "psf_models": {"standard": {"inference_config": "inference_standard.yaml"}} + } + + resources = parse_resources_config(config) + + assert resources.available["psf_models"]["standard"]["inference_config"] == ( + "inference_standard.yaml" + ) + + +def test_required_resources_must_be_a_list(): + with pytest.raises( + TypeError, + match="Required resources for metric 'goodness_of_fit' must be a list.", + ): + load_config("invalid/metric_required_resources_invalid_type.yaml") + + +def test_required_resources_element_must_be_a_str(): + with pytest.raises( + TypeError, + match="Required resources for metric 'goodness_of_fit' must contain only strings.", + ): + load_config("invalid/metric_required_resources_invalid_element.yaml") + + def test_metrics_minimal(): config = load_config("valid/metric_minimal.yaml") @@ -77,3 +169,104 @@ def test_reporting_configuration_must_be_mapping(): match="Reporting configuration must be a mapping", ): load_config("invalid/reporting_invalid_type.yaml") + + +# Tests for validation methods +def test_validate_metric_resources_all_valid(qc_config_factory): + with does_not_raise(): + validate_metric_resources(qc_config_factory()) + + +@pytest.mark.parametrize( + "required_resource", + [ + "psf_model_standard", + "psf_models.standard.foo", + "psf_models.", + ".standard", + ], +) +def test_validate_metric_resources_invalid_identifier( + qc_config_factory, + required_resource, +): + config = qc_config_factory(required_resources=[required_resource]) + + with pytest.raises( + ValueError, + match=( + f"Resource identifier '{required_resource}' must have the form " + "'.'." + ), + ): + validate_metric_resources(config) + + +@pytest.mark.parametrize( + "required_resource", + [ + "images.segmentation_maps", + "psf_models.imaginary", + ], +) +def test_validate_metric_resources_unknown_resource( + qc_config_factory, + required_resource, +): + config = qc_config_factory(required_resources=[required_resource]) + + with pytest.raises( + ValueError, + match=( + f"Metric 'goodness_of_fit' requires unknown resource '{required_resource}'." + ), + ): + validate_metric_resources(config) + + +def test_validate_rejection_policy_metrics_all_valid(qc_config_factory): + with does_not_raise(): + validate_rejection_policy_metrics(qc_config_factory()) + + +def test_validate_rejection_policy_metrics_metric_not_found(qc_config_factory): + config = qc_config_factory(rejection_metric="mask_obscuration") + + with pytest.raises( + ValueError, + match="Rejection policy configured for unknown metric 'mask_obscuration'.", + ): + validate_rejection_policy_metrics(config) + + +def test_validate_rejection_policy_metrics_metric_not_enabled(qc_config_factory): + metric = { + "goodness_of_fit": QualityMetricConfig( + enabled=False, + required_resources=[], + ) + } + + config = qc_config_factory(metrics=metric) + + with pytest.raises( + ValueError, + match="Rejection policy cannot be enabled because metric 'goodness_of_fit' is disabled.", + ): + validate_rejection_policy_metrics(config) + + +# Integration tests + + +def test_load_config_validates_configuration_pass(): + with does_not_raise(): + load_config("valid/quality_control.yaml") + + +def test_load_config_validates_configuration_raise_unknown_identifier(): + with pytest.raises( + ValueError, + match="Metric 'goodness_of_fit' requires unknown resource", + ): + load_config("invalid/metric_resource_identifier_unknown.yaml") diff --git a/src/wf_psf/tests/test_quality_control/data/invalid/metric_required_resources_invalid_element.yaml b/src/wf_psf/tests/test_quality_control/data/invalid/metric_required_resources_invalid_element.yaml new file mode 100644 index 00000000..6e91f023 --- /dev/null +++ b/src/wf_psf/tests/test_quality_control/data/invalid/metric_required_resources_invalid_element.yaml @@ -0,0 +1,6 @@ +metrics: + goodness_of_fit: + enabled: true + required_resources: + - psf_models.standard + - 123 \ No newline at end of file diff --git a/src/wf_psf/tests/test_quality_control/data/invalid/metric_required_resources_invalid_type.yaml b/src/wf_psf/tests/test_quality_control/data/invalid/metric_required_resources_invalid_type.yaml new file mode 100644 index 00000000..053a38d1 --- /dev/null +++ b/src/wf_psf/tests/test_quality_control/data/invalid/metric_required_resources_invalid_type.yaml @@ -0,0 +1,4 @@ +metrics: + goodness_of_fit: + enabled: true + required_resources: psf_models.standard \ No newline at end of file diff --git a/src/wf_psf/tests/test_quality_control/data/invalid/metric_resource_identifier_unknown.yaml b/src/wf_psf/tests/test_quality_control/data/invalid/metric_resource_identifier_unknown.yaml new file mode 100644 index 00000000..50023177 --- /dev/null +++ b/src/wf_psf/tests/test_quality_control/data/invalid/metric_resource_identifier_unknown.yaml @@ -0,0 +1,15 @@ +resources: + + psf_models: + standard: + inference_config: inference_standard.yaml + oversampled: + inference_config: inference_oversampled.yaml + +metrics: + + goodness_of_fit: + enabled: true + required_resources: + - images.segmentation_maps + diff --git a/src/wf_psf/tests/test_quality_control/data/invalid/rejection_invalid_params.yaml b/src/wf_psf/tests/test_quality_control/data/invalid/rejection_invalid_params.yaml deleted file mode 100644 index e2165bf8..00000000 --- a/src/wf_psf/tests/test_quality_control/data/invalid/rejection_invalid_params.yaml +++ /dev/null @@ -1,12 +0,0 @@ -# Future test data for validation of configuration settings -metrics: - goodness_of_fit: - enabled: true - inference_config: inference_config.yaml - model_cache: true - -rejection: - goodness_of_fit: - enabled: true - inference_config: - threshold: -1 \ No newline at end of file diff --git a/src/wf_psf/tests/test_quality_control/data/invalid/rejection_metric_not_enabled.yaml b/src/wf_psf/tests/test_quality_control/data/invalid/rejection_metric_not_enabled.yaml new file mode 100644 index 00000000..0dc25183 --- /dev/null +++ b/src/wf_psf/tests/test_quality_control/data/invalid/rejection_metric_not_enabled.yaml @@ -0,0 +1,22 @@ +resources: + + psf_models: + standard: + inference_config: inference_standard.yaml + oversampled: + inference_config: inference_oversampled.yaml + +metrics: + + mask_obscuration: + enabled: false + params: + aperture: gaussian + sigma: 2.5 + + +rejection: + + mask_obscuration: + enabled: true + threshold: 0.25 diff --git a/src/wf_psf/tests/test_quality_control/data/valid/goodness_of_fit_extended.yaml b/src/wf_psf/tests/test_quality_control/data/valid/goodness_of_fit_extended.yaml deleted file mode 100644 index b081e545..00000000 --- a/src/wf_psf/tests/test_quality_control/data/valid/goodness_of_fit_extended.yaml +++ /dev/null @@ -1,6 +0,0 @@ -metrics: - goodness_of_fit: - enabled: true - params: - inference_config: inference_config.yaml - model_cache: true \ No newline at end of file diff --git a/src/wf_psf/tests/test_quality_control/data/valid/missing_optional_sections.yaml b/src/wf_psf/tests/test_quality_control/data/valid/missing_optional_sections.yaml deleted file mode 100644 index 4a5fdba2..00000000 --- a/src/wf_psf/tests/test_quality_control/data/valid/missing_optional_sections.yaml +++ /dev/null @@ -1,3 +0,0 @@ -metrics: - mask_obscuration: - enabled: true \ No newline at end of file diff --git a/src/wf_psf/tests/test_quality_control/data/valid/quality_control.yaml b/src/wf_psf/tests/test_quality_control/data/valid/quality_control.yaml index 4486774a..2cbb7a15 100644 --- a/src/wf_psf/tests/test_quality_control/data/valid/quality_control.yaml +++ b/src/wf_psf/tests/test_quality_control/data/valid/quality_control.yaml @@ -1,12 +1,33 @@ +resources: + + psf_models: + standard: + inference_config: inference_standard.yaml + oversampled: + inference_config: inference_oversampled.yaml + metrics: mask_obscuration: enabled: true + params: + aperture: gaussian + sigma: 2.5 goodness_of_fit: enabled: true + required_resources: + - psf_models.standard + params: - inference_config: inference_config.yaml + statistic: reduced_chi_square + normalize_residuals: true + + shapes: + enabled: false + required_resources: + - psf_models.oversampled + rejection: