diff --git a/doc/changes/DM-54057.bugfix.md b/doc/changes/DM-54057.bugfix.md new file mode 100644 index 000000000..d07078b40 --- /dev/null +++ b/doc/changes/DM-54057.bugfix.md @@ -0,0 +1 @@ +Fix a Pydantic validation error reading `TaskMetadata` from provenance files, caused by incorrect JSON handling of NaN/inf. diff --git a/python/lsst/pipe/base/_task_metadata.py b/python/lsst/pipe/base/_task_metadata.py index 028403224..da201aca1 100644 --- a/python/lsst/pipe/base/_task_metadata.py +++ b/python/lsst/pipe/base/_task_metadata.py @@ -37,9 +37,18 @@ import numbers import sys from collections.abc import Collection, Iterator, Mapping, Sequence -from typing import Any, Protocol - -from pydantic import BaseModel, ConfigDict, Field, StrictBool, StrictFloat, StrictInt, StrictStr +from typing import Annotated, Any, Protocol + +from pydantic import ( + BaseModel, + BeforeValidator, + ConfigDict, + Field, + StrictBool, + StrictFloat, + StrictInt, + StrictStr, +) # The types allowed in a Task metadata field are restricted # to allow predictable serialization. @@ -110,6 +119,20 @@ class GetSetDictMetadata(SetDictMetadata, GetDictMetadata, Protocol): """ +# Some TaskMetadata JSON representations have been written (provenance files +# for the DP2 production) with NaNs converted to JSON null rather than a +# constant "inf" or "nan", since the Pydantic model_config ser_json_inf_nan +# doesn't automatically get picked up by parent models. This before-validator +# turns those nulls back into NaNs, making the metadata readable again. +def _convert_null_to_nan(value: Any) -> float: + if value is None: + return float("nan") + return float(value) + + +type _NullToNanFloat = Annotated[StrictFloat, BeforeValidator(_convert_null_to_nan)] + + class TaskMetadata(BaseModel): """Dict-like object for storing task metadata. @@ -124,11 +147,12 @@ class TaskMetadata(BaseModel): """ # Pipelines regularly generate NaN and Inf so these need to be - # supported even though that's a JSON extension. + # supported even though that's a JSON extension. Note that any parent + # models that might hold a TaskMetadata also need to set this explicitly! model_config = ConfigDict(ser_json_inf_nan="constants") - scalars: dict[str, StrictFloat | StrictInt | StrictBool | StrictStr] = Field(default_factory=dict) - arrays: dict[str, list[StrictFloat] | list[StrictInt] | list[StrictBool] | list[StrictStr]] = Field( + scalars: dict[str, _NullToNanFloat | StrictInt | StrictBool | StrictStr] = Field(default_factory=dict) + arrays: dict[str, list[_NullToNanFloat] | list[StrictInt] | list[StrictBool] | list[StrictStr]] = Field( default_factory=dict ) metadata: dict[str, "TaskMetadata"] = Field(default_factory=dict) diff --git a/python/lsst/pipe/base/quantum_graph/_provenance.py b/python/lsst/pipe/base/quantum_graph/_provenance.py index 03523f026..62603a3f2 100644 --- a/python/lsst/pipe/base/quantum_graph/_provenance.py +++ b/python/lsst/pipe/base/quantum_graph/_provenance.py @@ -507,6 +507,11 @@ class ProvenanceTaskMetadataModel(pydantic.BaseModel): file. """ + # We want to convert infs and nans to constants, not null. Unfortunately + # the fact that TaskMetadata _also_ sets this is ignored when that model + # is nested here. + model_config = pydantic.ConfigDict(ser_json_inf_nan="constants") + attempts: list[TaskMetadata | None] = pydantic.Field(default_factory=list) """Metadata from attempts to run this task, ordered chronologically from first to last. diff --git a/tests/data/DM-54057.json b/tests/data/DM-54057.json new file mode 100644 index 000000000..be8df370d --- /dev/null +++ b/tests/data/DM-54057.json @@ -0,0 +1,114 @@ +{ + "attempts": [ + { + "scalars": {}, + "arrays": {}, + "metadata": { + "calibrateImage": { + "scalars": { + "__version__": 1, + "nodeName": "sdfmilan185" + }, + "arrays": {}, + "metadata": {} + }, + "calibrateImage:psf_measure_psf": { + "scalars": { + "__version__": 1, + "nodeName": "sdfmilan185", + "spatialFitChi2": null, + "numAvailStars": 300, + "numGoodStars": 268, + "avgX": 1983.4188094399349, + "avgY": 2164.4395513319205 + }, + "arrays": {}, + "metadata": {} + }, + "quantum": { + "scalars": { + "__version__": 1, + "nodeName": "sdfmilan185", + "caveats": 0 + }, + "arrays": { + "prepUtc": ["2026-02-05T17:52:57.668196+00:00"], + "prepCpuTime": [146.985167419], + "prepUserTime": [142.67557299999999], + "prepSystemTime": [4.309589], + "prepMaxResidentSetSize": [3160428544], + "prepMinorPageFaults": [1056116], + "prepMajorPageFaults": [305], + "prepBlockInputs": [1210784], + "prepBlockOutputs": [667896], + "prepVoluntaryContextSwitches": [31863], + "prepInvoluntaryContextSwitches": [421], + "initUtc": ["2026-02-05T17:52:57.691501+00:00"], + "initCpuTime": [146.999102404], + "initUserTime": [142.68556], + "initSystemTime": [4.313538], + "initMaxResidentSetSize": [3160428544], + "initMinorPageFaults": [1056940], + "initMajorPageFaults": [305], + "initBlockInputs": [1210784], + "initBlockOutputs": [667904], + "initVoluntaryContextSwitches": [31974], + "initInvoluntaryContextSwitches": [421], + "startUtc": ["2026-02-05T17:52:57.697625+00:00"], + "startCpuTime": [147.00516647], + "startUserTime": [142.69162], + "startSystemTime": [4.313542], + "startMaxResidentSetSize": [3160428544], + "startMinorPageFaults": [1057474], + "startMajorPageFaults": [305], + "startBlockInputs": [1210784], + "startBlockOutputs": [667904], + "startVoluntaryContextSwitches": [31974], + "startInvoluntaryContextSwitches": [421], + "endUtc": ["2026-02-05T17:54:36.235890+00:00"], + "endCpuTime": [244.109828766], + "endUserTime": [238.41773], + "endSystemTime": [5.692091], + "endMaxResidentSetSize": [3160428544], + "endMinorPageFaults": [1498257], + "endMajorPageFaults": [305], + "endBlockInputs": [1393576], + "endBlockOutputs": [882504], + "endVoluntaryContextSwitches": [32434], + "endInvoluntaryContextSwitches": [582], + "outputs": [ + "019c2ec7-90d0-72d0-8112-ff971c62524e", + "019c2ec7-90d0-770e-86c3-7dd2e93d155b", + "019c2ec7-90d0-7500-ab1f-ec6311246eff", + "019c2ec7-90d0-7a8b-9df7-3e5d04b6fcad", + "019c2ec7-90d0-77eb-9308-78373e7c5fc5", + "019c2ec7-90d0-7536-895b-0402634a3f8d", + "019c2ec7-90d0-798e-a38d-ad622cadeb7f", + "019c2ec7-90d0-78c7-b7fe-c3a75883b17b", + "019c2ec7-90d0-71ab-883e-e36fa8ce9913" + ] + }, + "metadata": { + "butler_metrics": { + "scalars": { + "time_in_put": 3.5570390224456787, + "time_in_get": 1.2412796020507812, + "time_in_ingest": 0.0, + "n_get": 8, + "n_put": 9, + "n_ingest": 0 + }, + "arrays": {}, + "metadata": {} + } + } + }, + "job": { + "scalars": { "qg_read_time": 4.202885150909424, "qg_size": 6 }, + "arrays": {}, + "metadata": {} + } + } + } + ] +} diff --git a/tests/test_aggregator.py b/tests/test_aggregator.py index 0192b62cd..cd9dc73b3 100644 --- a/tests/test_aggregator.py +++ b/tests/test_aggregator.py @@ -72,6 +72,7 @@ ProvenanceQuantumInfo, ProvenanceQuantumReport, ProvenanceReport, + ProvenanceTaskMetadataModel, ) from lsst.pipe.base.quantum_graph.aggregator import AggregatorConfig, FatalWorkerError, aggregate_graph from lsst.pipe.base.quantum_graph.ingest_graph import ingest_graph @@ -1330,6 +1331,15 @@ def check( data_id_table_dir="dir1", ) + def test_bad_metadata_readable(self) -> None: + """Test that consolidated metadata accidentally written with floats + transformed to JSON null are now readable. + """ + with open(os.path.join(os.path.dirname(__file__), "data", "DM-54057.json")) as stream: + data = stream.read() + prov_md = ProvenanceTaskMetadataModel.model_validate_json(data) + self.assertTrue(np.isnan(prov_md.attempts[0]["calibrateImage:psf_measure_psf"]["spatialFitChi2"])) + if __name__ == "__main__": lsst.utils.tests.init()