diff --git a/changelog.d/accept-regional-model-mismatch.added.md b/changelog.d/accept-regional-model-mismatch.added.md new file mode 100644 index 00000000..a60f8500 --- /dev/null +++ b/changelog.d/accept-regional-model-mismatch.added.md @@ -0,0 +1 @@ +`certify-data --accept-regional-model-mismatch REASON` (UK only) keeps a local-area release routed when it was built on another policyengine-uk version, and records the reviewed mismatch as `regional_certification` in the bundle instead of refusing; area receipts are uncertified. diff --git a/docs/engineering/skills/data-certification.md b/docs/engineering/skills/data-certification.md index 23c5d813..1ce62d88 100644 --- a/docs/engineering/skills/data-certification.md +++ b/docs/engineering/skills/data-certification.md @@ -79,6 +79,31 @@ Microcosm national release with one of: claim first. Its `frs_2024_25` is not carried, so keep it loadable with a `dataset_overlays.uk` entry. +- **An older local-area release, on an accepted mismatch.** When the national + cut moves to a new policyengine-uk before a local-area cut on that version + exists, a reviewer can keep the current local-area release routed instead of + dropping area regions: + + ```bash + python scripts/bundle.py certify-data --country uk --data-producer populace \ + --manifest-uri "hf://dataset/policyengine/populace-uk-private@/releases/microcosm-uk-2024-25-national/release_manifest.json" \ + --regional-manifest-uri "" \ + --accept-regional-model-mismatch "" \ + --model-version "" + ``` + + Certification then records `regional_certification` in the bundle: + `compatibility_basis: accepted_model_mismatch`, both model versions, the + reason and `area_receipts: uncertified`. Area receipts are uncertified in any + case, because the certificate covers the national dataset only (#685). + The cost is real and goes in the pull request. The local-area file was built + and calibrated under the older model. policyengine-core ignores, with a + warning, the columns the newer model no longer defines, and inputs the newer + model added take their defaults in area runs. The flag is UK only. It refuses + an empty reason, a regional release that already covers the version (so no + certificate records a mismatch that is not there) and a run without a + regional release. The core must still be covered. + Either way, certification checks that the regional release serves the pinned model by its own build or compatibility claims (a `--qualified-pair-id` names the primary release only), checks its core claims as for the primary release diff --git a/scripts/bundle.py b/scripts/bundle.py index 53b74a26..5530a0a7 100644 --- a/scripts/bundle.py +++ b/scripts/bundle.py @@ -48,6 +48,10 @@ def _certify_data(args: argparse.Namespace) -> int: argv.extend(["--regional-release-kind", args.regional_release_kind]) if args.drop_regional_release: argv.append("--drop-regional-release") + if args.accept_regional_model_mismatch: + argv.extend( + ["--accept-regional-model-mismatch", args.accept_regional_model_mismatch] + ) if args.no_generate: argv.append("--no-generate") if args.no_changelog: @@ -226,6 +230,15 @@ def _parser() -> argparse.ArgumentParser: "Cannot be combined with --regional-manifest-uri." ), ) + certify.add_argument( + "--accept-regional-model-mismatch", + metavar="REASON", + help=( + "UK only: route regions to a regional release on another model " + "version, recording this reviewed reason; forwarded to " + "scripts/certify_data_release.py." + ), + ) certify.add_argument( "--regional-artifact-prefix", help="Legacy per-state artifact prefix to import. Defaults to states/.", diff --git a/scripts/certify_data_release.py b/scripts/certify_data_release.py index f3a720ee..5c3bf687 100644 --- a/scripts/certify_data_release.py +++ b/scripts/certify_data_release.py @@ -115,6 +115,15 @@ def main(argv=None) -> int: "qualification of this pair (requires --core-version)." ), ) + parser.add_argument( + "--accept-regional-model-mismatch", + metavar="REASON", + help=( + "UK only: route regions to a regional release that does not cover " + "the pinned model, recording this reviewed reason in the bundle " + "instead of refusing. The core must still be covered." + ), + ) parser.add_argument("--no-generate", action="store_true") parser.add_argument("--no-changelog", action="store_true") parser.add_argument( @@ -149,6 +158,7 @@ def main(argv=None) -> int: drop_regional_release=args.drop_regional_release, core_version=args.core_version, qualified_pair_id=args.qualified_pair_id, + regional_model_mismatch_reason=args.accept_regional_model_mismatch, ) print(result.summary()) diff --git a/src/policyengine/provenance/certification.py b/src/policyengine/provenance/certification.py index dc773e21..d42c0ca1 100644 --- a/src/policyengine/provenance/certification.py +++ b/src/policyengine/provenance/certification.py @@ -410,6 +410,56 @@ def model_compatibility_basis( return basis, [] if warning is None else [warning] +#: A regional release's model coverage accepted by a reviewer, not by the +#: release's own build or claims (``--accept-regional-model-mismatch``). +REGIONAL_BASIS_ACCEPTED_MISMATCH = "accepted_model_mismatch" + + +def accepted_regional_mismatch( + regional_manifest: DataReleaseManifest, + model_package: str, + model_version: str, + reason: str, +) -> tuple[dict, str]: + """Record a reviewed decision to route regions to a release on another model. + + Area runs on such a release are uncertified twice over: the certificate + covers the national dataset only, and the regional file was built and + calibrated under another model version. The record names both versions + and the reason so the bundle, its TRO and every consumer can say so. + Returns ``(record, warning)``. + """ + + reason = reason.strip() + if not reason: + raise CertificationError( + "--accept-regional-model-mismatch needs a reason a reviewer can read." + ) + built = ( + regional_manifest.build.built_with_model_package + if (regional_manifest.build is not None) + else None + ) + built_with = ( + built.version if built is not None and built.name == model_package else None + ) + record = { + "compatibility_basis": REGIONAL_BASIS_ACCEPTED_MISMATCH, + "certified_for_model_version": model_version, + "built_with_model_version": built_with, + "accepted_reason": reason, + "area_receipts": "uncertified", + } + warning = ( + f"regional release: built with {model_package} " + f"{built_with or 'an unrecorded version'} and routed under " + f"{model_version} on an accepted mismatch ({reason}); area runs are " + "uncertified, inputs the newer model added take their defaults there, " + "and columns it dropped are ignored on load" + ) + return record, warning + + def _check_core_compatibility( manifest: DataReleaseManifest, core_version: str, @@ -1107,6 +1157,7 @@ def build_bundle_data_release_payload( regional_manifest_uri: Optional[str] = None, regional_uri_parts: Optional[dict] = None, regional_release_kind: Optional[RegionalReleaseKind] = None, + regional_certification: Optional[dict] = None, ) -> dict: """Map the country certification payload into bundle ``data_releases``.""" payload = copy.deepcopy(country_payload) @@ -1126,6 +1177,8 @@ def build_bundle_data_release_payload( ) if regional_release_kind is not None: payload["regional_release_kind"] = regional_release_kind + if regional_certification is not None: + payload["regional_certification"] = regional_certification payload["default_dataset_uri"] = certified_artifact.get("uri") return payload @@ -1149,6 +1202,7 @@ def certify( core_version: Optional[str] = None, qualified_pair_id: Optional[str] = None, bundle_core_version: Optional[str] = None, + regional_model_mismatch_reason: Optional[str] = None, ) -> tuple[dict, str, list[str]]: raise NotImplementedError @@ -1172,6 +1226,7 @@ def certify( core_version: Optional[str] = None, qualified_pair_id: Optional[str] = None, bundle_core_version: Optional[str] = None, + regional_model_mismatch_reason: Optional[str] = None, ) -> tuple[dict, str, list[str]]: raise CertificationError( "Legacy data-producer certification updates are not implemented in " @@ -1198,6 +1253,7 @@ def certify( core_version: Optional[str] = None, qualified_pair_id: Optional[str] = None, bundle_core_version: Optional[str] = None, + regional_model_mismatch_reason: Optional[str] = None, ) -> tuple[dict, str, list[str]]: manifest, manifest_sha256, uri_parts = fetch_release_manifest( manifest_uri, token=token @@ -1208,6 +1264,18 @@ def certify( recorded_release_kind = None regional_artifacts: dict[str, DataReleaseArtifact] = {} regional_warnings: list[str] = [] + regional_certification: Optional[dict] = None + mismatch_warning: Optional[str] = None + if regional_model_mismatch_reason is not None and regional_manifest_uri is None: + raise CertificationError( + "--accept-regional-model-mismatch applies to a regional release; " + "pass --regional-manifest-uri." + ) + if regional_model_mismatch_reason is not None and country != "uk": + raise CertificationError( + "--accept-regional-model-mismatch is only supported for the UK, " + "whose area receipts are uncertified." + ) if regional_manifest_uri is not None: regional_manifest, _, regional_uri_parts = fetch_release_manifest( regional_manifest_uri, token=token @@ -1219,6 +1287,27 @@ def certify( _, regional_warnings = model_compatibility_basis( regional_manifest, model_package, model_version ) + except CertificationError as exc: + if regional_model_mismatch_reason is None: + raise CertificationError( + f"Regional release {regional_manifest_uri}: {exc}" + ) from exc + regional_certification, mismatch_warning = accepted_regional_mismatch( + regional_manifest, + model_package, + model_version, + regional_model_mismatch_reason, + ) + regional_warnings = [] + else: + if regional_model_mismatch_reason is not None: + raise CertificationError( + f"Regional release {regional_manifest_uri} serves " + f"{model_package} {model_version} by its own build or " + "claims; drop --accept-regional-model-mismatch so the " + "certificate does not record a mismatch that is not there." + ) + try: if core_version is not None: _check_core_compatibility( regional_manifest, core_version, None, regional_warnings @@ -1230,6 +1319,8 @@ def certify( regional_warnings = [ f"regional release: {warning}" for warning in regional_warnings ] + if mismatch_warning is not None: + regional_warnings.append(mismatch_warning) primary_artifact_names = set(manifest.artifacts) manifest, recorded_release_kind = merge_regional_release_manifest( country, @@ -1323,6 +1414,7 @@ def certify( regional_manifest_uri=regional_manifest_uri, regional_uri_parts=regional_uri_parts, regional_release_kind=recorded_release_kind, + regional_certification=regional_certification, ), manifest_sha256, warnings, @@ -1368,6 +1460,7 @@ def certify_data_release( drop_regional_release: bool = False, core_version: Optional[str] = None, qualified_pair_id: Optional[str] = None, + regional_model_mismatch_reason: Optional[str] = None, ) -> CertificationResult: """Certify a data release into the bundle manifest. @@ -1383,6 +1476,12 @@ def certify_data_release( given, by its own build or claims: a qualified pair names the primary release only. While the bundle routes regions, a run without ``regional_manifest_uri`` is refused unless ``drop_regional_release``. + + ``regional_model_mismatch_reason`` (UK only) records a reviewer's + acceptance of a regional release that does not cover the pinned model, + instead of refusing it. The core must still be covered. The bundle then + carries ``regional_certification`` naming both model versions and the + reason; area receipts are uncertified either way. """ certified_country = _certified_country(country) if drop_regional_release and regional_manifest_uri is not None: @@ -1429,6 +1528,7 @@ def certify_data_release( bundle_core_version=( (bundle.get("packages") or {}).get("policyengine-core") or {} ).get("version"), + regional_model_mismatch_reason=regional_model_mismatch_reason, ) previous_release = (bundle.get("data_releases") or {}).get(country) or {} bundle.setdefault("data_releases", {})[country] = data_release diff --git a/src/policyengine/provenance/manifest.py b/src/policyengine/provenance/manifest.py index f3b15fb5..328db86e 100644 --- a/src/policyengine/provenance/manifest.py +++ b/src/policyengine/provenance/manifest.py @@ -202,6 +202,21 @@ class CertifiedDataArtifact(BaseModel): RegionalReleaseKind = Literal["local_area", "area_coded_national"] +class RegionalCertification(BaseModel): + """A reviewer's acceptance of a regional release on another model version. + + Written only by ``certify-data --accept-regional-model-mismatch``: the + regional release neither was built with nor claims the certified model, + and area runs on it are uncertified. + """ + + compatibility_basis: Literal["accepted_model_mismatch"] + certified_for_model_version: str + built_with_model_version: Optional[str] = None + accepted_reason: str + area_receipts: Literal["uncertified"] = "uncertified" + + class CountryReleaseManifest(BaseModel): schema_version: int = 1 bundle_id: Optional[str] = None @@ -218,6 +233,9 @@ class CountryReleaseManifest(BaseModel): ``local_area`` for a non-default local-area release, and ``area_coded_national`` for another release's national dataset that carries area codes (and its area weight matrices).""" + regional_certification: Optional[RegionalCertification] = None + """Present only when the regional release was routed on an accepted + model mismatch rather than its own build or claims.""" certified_data_artifact: Optional[CertifiedDataArtifact] = None certification: Optional[DataCertification] = None source_sha256: Optional[str] = Field(default=None, exclude=True) diff --git a/tests/test_certify_data_release.py b/tests/test_certify_data_release.py index 092420f6..a8c8b5b2 100644 --- a/tests/test_certify_data_release.py +++ b/tests/test_certify_data_release.py @@ -1412,6 +1412,150 @@ def test__given_regional_release_without_a_claim__then_refuses(self, tmp_path): regional_release_kind="area_coded_national", ) + def _older_dense(self) -> dict: + dense = _microcosm_uk_dense_payload() + dense["build"]["built_with_model_package"]["version"] = "2.120.0" + dense["compatible_model_packages"] = [ + {"name": "policyengine-uk", "specifier": "==2.120.0"} + ] + return dense + + def test__given_older_dense_cut__then_refuses_without_an_accepted_mismatch( + self, tmp_path + ): + with pytest.raises(CertificationError, match="Regional release .*2.126.12"): + _certify_uk( + _tmp_bundle(tmp_path), + [ + _response(_microcosm_uk_national_payload()), + _response(self._older_dense()), + ], + regional_manifest_uri=UK_DENSE_URI, + ) + + def test__given_accepted_mismatch__then_routes_and_records_it(self, tmp_path): + bundle_path = _tmp_bundle(tmp_path) + reason = "area receipts are uncertified; the dense cut lags one release" + + result = _certify_uk( + bundle_path, + [ + _response(_microcosm_uk_national_payload()), + _response(self._older_dense()), + ], + regional_manifest_uri=UK_DENSE_URI, + regional_model_mismatch_reason=reason, + ) + + release = json.loads(bundle_path.read_text())["data_releases"]["uk"] + assert release["regional_release_kind"] == "local_area" + assert release["regional_certification"] == { + "compatibility_basis": "accepted_model_mismatch", + "certified_for_model_version": UK_MODEL_VERSION, + "built_with_model_version": "2.120.0", + "accepted_reason": reason, + "area_receipts": "uncertified", + } + # The national certificate itself is untouched by the regional record. + assert release["certification"]["compatibility_basis"] == ( + "built_with_model_package" + ) + assert _sim_api_routes_both_area_types(release) + manifest = CountryReleaseManifest.model_validate(release) + assert manifest.regional_certification is not None + assert manifest.regional_certification.built_with_model_version == "2.120.0" + mismatch = [w for w in result.warnings if "accepted mismatch" in w] + assert len(mismatch) == 1 and "2.120.0" in mismatch[0] + + def test__given_a_covering_regional_release__then_an_accepted_mismatch_refuses( + self, tmp_path + ): + with pytest.raises(CertificationError, match="drop --accept-regional"): + _certify_uk( + _tmp_bundle(tmp_path), + [ + _response(_microcosm_uk_national_payload()), + _response(_microcosm_uk_dense_payload()), + ], + regional_manifest_uri=UK_DENSE_URI, + regional_model_mismatch_reason="not needed", + ) + + @pytest.mark.parametrize("reason", ["", " "]) + def test__given_an_empty_reason__then_refuses(self, tmp_path, reason): + with pytest.raises(CertificationError, match="needs a reason"): + _certify_uk( + _tmp_bundle(tmp_path), + [ + _response(_microcosm_uk_national_payload()), + _response(self._older_dense()), + ], + regional_manifest_uri=UK_DENSE_URI, + regional_model_mismatch_reason=reason, + ) + + def test__given_an_accepted_mismatch__then_the_core_must_still_be_covered( + self, tmp_path + ): + national = _microcosm_uk_national_payload() + national["compatible_core_packages"] = [ + {"name": "policyengine-core", "specifier": "==3.33.0"} + ] + with pytest.raises( + CertificationError, match="Regional release .*policyengine-core 3.33.0" + ): + _certify_uk( + _tmp_bundle(tmp_path), + [_response(national), _response(self._older_dense())], + regional_manifest_uri=UK_DENSE_URI, + regional_model_mismatch_reason="area receipts are uncertified", + core_version="3.33.0", + ) + + def test__given_an_accepted_mismatch_without_a_regional_release__then_refuses( + self, tmp_path + ): + with pytest.raises(CertificationError, match="pass --regional-manifest-uri"): + _certify_uk( + _tmp_bundle(tmp_path), + [_response(_microcosm_uk_national_payload())], + drop_regional_release=True, + regional_model_mismatch_reason="area receipts are uncertified", + ) + + def test__given_bundle_wrapper_accepted_mismatch__then_forwards_it( + self, monkeypatch + ): + bundle_script = _load_bundle_script(monkeypatch) + captured: dict[str, list[str]] = {} + fake = ModuleType("certify_data_release") + + def fake_main(argv): + captured["argv"] = argv + return 0 + + fake.main = fake_main + monkeypatch.setitem(sys.modules, "certify_data_release", fake) + reason = "area receipts are uncertified" + assert ( + bundle_script.main( + [ + "certify-data", + "--country", + "uk", + "--manifest-uri", + UK_NATIONAL_URI, + "--regional-manifest-uri", + UK_DENSE_URI, + "--accept-regional-model-mismatch", + reason, + ] + ) + == 0 + ) + argv = captured["argv"] + assert argv[argv.index("--accept-regional-model-mismatch") + 1] == reason + def test__given_pinned_core__then_the_regional_release_must_cover_it( self, tmp_path ):