Fabricate CMIP7 coverage by repeating the final year - #887
Conversation
The CMIP6 source for a fabricated CMIP7 series stops in 2014, so the series was relabelled onto the requested end date. This moved every real year with it, and each file was relabelled on its own, so a dataset split across several chunks ended up with overlapping files that all finished on the same month. Replaces the relabelling with padding. The real timesteps keep their dates and values, and the final year is tiled forward until the series reaches the requested end. Re-mints the fire and cloud radiative effects CMIP7 baselines, whose fabricated files now start in 1850 rather than 1857.
📝 WalkthroughWalkthroughCMIP7 historical extension now repeats the final complete year instead of relabelling the full time axis. Only the latest temporal file is extended. Unit tests, catalogues, manifests, regression data, and related documentation were updated. ChangesCMIP7 historical padding
Sequence Diagram(s)sequenceDiagram
participant fetch_datasets
participant _latest_file
participant _convert_file_to_cmip7
participant repeat_final_year_to
fetch_datasets->>_latest_file: inspect source files
_latest_file-->>fetch_datasets: return latest temporal file
fetch_datasets->>_convert_file_to_cmip7: convert each source file
_convert_file_to_cmip7->>repeat_final_year_to: extend latest file to target month
repeat_final_year_to-->>_convert_file_to_cmip7: return padded dataset
Merge Risk: 🟡 Moderate · up to The PR changes CMIP7 coverage generation to repeat the final year, but the current implementation can fabricate values into incorrect months, return incomplete data when validation fails, or abort a request when a source file is unreadable. These correctness and availability risks should be fixed or explicitly accepted before merge. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 87.18% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 39 functions across 5 files. (6 skipped: 6 unsupported.) Full details: Description checkExplanation The description clearly explains the implementation, affected datasets, edge cases, test impact, and changelog update. It does not use the required "## Description" and "## Checklist" headings or confirm the checklist items explicitly, but the required information is mostly present. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
…y label Padding every file in a dataset ran each chunk out to the same end month, so a dataset split across several files still produced overlapping converted files. Only the file holding the final timestep is extended now. Simplifies the padding itself. The input is always monthly, so the final year is selected by label and each repeat is that year with its year stamp moved. This drops the month arithmetic, the day clamping, and the partial-year tiling. A series that does not end in December, or whose final year is incomplete, now fails rather than producing a gap.
Adds the end-to-end check that only the file ending last is extended, which is what the overlapping converted files came down to. Falls back a day when a leap day is restamped onto a common year, rather than letting cftime raise from inside the relabelling.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/climate-ref-core/src/climate_ref_core/esgf/cmip7.py (1)
352-354: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDo not suppress final-year validation errors.
When extension is requested and the latest file ends outside December or has an incomplete final year,
repeat_final_year_toraisesValueError. Lines 352-354 catch the error and continue. If an earlier chunk converted, this method returns that incomplete dataset row instead of failing the request. Re-raise the helper validation errors.Proposed fix
+ except (TypeError, ValueError): + raise except Exception as e: logger.exception(f"Failed to convert {cmip6_path.name}: {e}") continue
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: feefc726-ec16-4852-b8e4-ec9ff4bfaf85
📒 Files selected for processing (11)
changelog/887.fix.mdpackages/climate-ref-core/src/climate_ref_core/cmip6_to_cmip7.pypackages/climate-ref-core/src/climate_ref_core/esgf/cmip7.pypackages/climate-ref-core/tests/unit/esgf/test_cmip7.pypackages/climate-ref-core/tests/unit/test_cmip6_to_cmip7.pypackages/climate-ref-esmvaltool/src/climate_ref_esmvaltool/diagnostics/climate_drivers_for_fire.pypackages/climate-ref-esmvaltool/tests/test-data/climate-drivers-for-fire/cmip7/catalog.yamlpackages/climate-ref-esmvaltool/tests/test-data/climate-drivers-for-fire/cmip7/manifest.jsonpackages/climate-ref-esmvaltool/tests/test-data/cloud-radiative-effects/cmip7/catalog.yamlpackages/climate-ref-esmvaltool/tests/test-data/cloud-radiative-effects/cmip7/manifest.jsonpackages/climate-ref-esmvaltool/tests/test-data/cloud-radiative-effects/cmip7/regression/series.json
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.
| if len(final_year["time"]) != _MONTHS_PER_YEAR: | ||
| raise ValueError(f"A full final year is needed to repeat, got {len(final_year['time'])} months") |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Validate the complete January-to-December month sequence.
Line 572 only checks for 12 records. A final year with a duplicate month and a missing month passes this check, then repeats values into the wrong fabricated months. Verify that the selected timestamps contain each month from January through December exactly once. Add a regression test for this input.
Proposed fix
final_year = ds.sel(time=str(last.year))
-if len(final_year["time"]) != _MONTHS_PER_YEAR:
+final_year_months = [_month_index(t) for t in final_year["time"].values]
+expected_months = list(range(last.year * _MONTHS_PER_YEAR, (last.year + 1) * _MONTHS_PER_YEAR))
+if final_year_months != expected_months:
raise ValueError(f"A full final year is needed to repeat, got {len(final_year['time'])} months")1 participant
Reworks how fabricated CMIP7 test data reaches years the CMIP6 source never ran. The series used to be relabelled onto the requested end date, which dragged every real year along with it. It is now padded instead, by repeating the final year until it reaches that date, so the real timesteps keep their real dates and values.
Two things worth a close look.
tozis 1850-1949 plus 1950-2014) produced overlapping files that all ended on the same month. Padding only ever touches the tail, so that goes away.Re-mints the two affected CMIP7 baselines. Their fabricated files move from
185701-202112to185001-202112.The ozone diagnostics need this to ask for a 1950 start on CMIP7, so #886 should land after it.
Summary by CodeRabbit
Bug Fixes
Tests
Documentation