Skip to content

Improve parent experiment support #214

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

Improve parent experiment support #214

wants to merge 11 commits into from

Conversation

@bouweandela
Copy link
Contributor

@bouweandela bouweandela commented Mar 28, 2025 •
edited
Loading

Description

Improve parent experiment support. Additional sample data for ECS in Climate-REF/ref-sample-data#54.

Closes #212

Checklist

Please confirm that this pull request has done the following:

  • Tests added
  • Documentation added (where applicable)
  • Changelog item added to changelog/

@bouweandela bouweandela force-pushed the improve-parent-experiment-support branch 2 times, most recently from 128fcb5 to f22ff5d Compare September 24, 2025 15:36
@lewisjared lewisjared moved this to Backlog in REF V1 Sep 26, 2025
@lewisjared lewisjared moved this from Backlog to In progress in REF V1 Sep 26, 2025
@bouweandela bouweandela force-pushed the improve-parent-experiment-support branch 4 times, most recently from 5b664c3 to 852357e Compare October 1, 2025 20:05
@codecov
Copy link

codecov bot commented Oct 1, 2025 •
edited
Loading

Codecov Report

❌ Patch coverage is 98.61111% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...imate-ref-core/src/climate_ref_core/constraints.py 96.77% 0 Missing and 1 partial ⚠️
Files with missing lines Coverage Δ
...tool/src/climate_ref_esmvaltool/diagnostics/ecs.py 100.00% <100.00%> (ø)
...tool/src/climate_ref_esmvaltool/diagnostics/tcr.py 100.00% <100.00%> (ø)
...ool/src/climate_ref_esmvaltool/diagnostics/tcre.py 100.00% <100.00%> (ø)
...tool/src/climate_ref_esmvaltool/diagnostics/zec.py 100.00% <100.00%> (ø)
...ef-esmvaltool/src/climate_ref_esmvaltool/recipe.py 95.06% <100.00%> (+2.07%) ⬆️
packages/climate-ref/src/climate_ref/testing.py 63.26% <100.00%> (ø)
...imate-ref-core/src/climate_ref_core/constraints.py 96.48% <96.77%> (+0.37%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@bouweandela
Add constraint to select parent dataset
b53e175 
Fix issues where only the first file from a dataset was selected
@bouweandela bouweandela force-pushed the improve-parent-experiment-support branch from 7780648 to 7a82387 Compare October 2, 2025 08:29
@bouweandela bouweandela force-pushed the improve-parent-experiment-support branch from 7a82387 to 5a3da8d Compare October 2, 2025 20:23
@bouweandela bouweandela force-pushed the improve-parent-experiment-support branch from cf38f1c to dc7c7aa Compare October 2, 2025 20:44
@bouweandela bouweandela marked this pull request as ready for review October 2, 2025 20:56
@bouweandela bouweandela requested a review from lewisjared October 2, 2025 20:56
lewisjared
lewisjared reviewed Oct 3, 2025
View reviewed changes
Copy link
Contributor

@lewisjared lewisjared left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice. I'll do a dry run solve to see how many datasets we lose with the data I have available unless you have already done that?

packages/climate-ref-core/src/climate_ref_core/constraints.py Outdated
# TODO: update for CMIP7
]
for parent_facet_map in parent_facet_options:
# We do not have access to the SourceDatasetType so we need to
Copy link
Contributor

@lewisjared lewisjared Oct 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could make it an argument to apply(). I haven't looked at the call sites, but would likely be possible. Might be needed for the above callout to CMIP7

Copy link
Contributor Author

@bouweandela bouweandela Oct 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could make it an instance attribute, but then the constraint would need to be created as AddParentDataset(SourceDatasetType.CMIP6). Would you prefer that?

Copy link
Contributor

@lewisjared lewisjared Oct 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup good idea

Copy link
Contributor Author

@bouweandela bouweandela Oct 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 16ea2a2

@bouweandela
Copy link
Contributor Author

bouweandela commented Oct 3, 2025

I'll do a dry run solve to see how many datasets we lose with the data I have available unless you have already done that?

That would be great, I haven't done that.

bouweandela and others added 4 commits October 6, 2025 10:11
@bouweandela
Add factory method and more readable implementation
16ea2a2
@bouweandela
Update solver test
0d75824
@lewisjared
Merge remote-tracking branch 'origin/main' into improve-parent-experi…
c92a7ed 
…ment-support

* origin/main: (27 commits)
  chore: Rename changelog
  Update packages/climate-ref-pmp/src/climate_ref_pmp/diagnostics/annual_cycle.py
  Update reference output
  Add reference values to series output
  minor fixes
  add docs for the configrations of HPCExecutor validation
  minor cleanup on hpc.py
  add NPGO regression test output files
  fix broken test for annual cycle
  Lee1043 pmp mov bugfix (Climate-REF#450)
  add test results
  clean up
  clean up
  clean up
  clean up
  add change log
  clean up
  update
  update
  in progress
  ...
@lewisjared
Merge remote-tracking branch 'origin/main' into improve-parent-experi…
b630bce 
…ment-support

* origin/main:
  Update reference output
  Convert default fillvalues to NaNs
  chore: Changelog
  chore: Catch exception in local
  chore: Minor tweaks to config
  test: Fix test
  fix: Skip missing directories and continue
  chore: Add logs for dry-mode
  chore: Add log message for the number of solves
@lewisjared
Copy link
Contributor

lewisjared commented Oct 7, 2025

I didn't get the chance to rerun the regression tests as my internet is pretty bad. I might have to leave this out of v1.0.0 and do a quick follow up release this evening

@lewisjared
Copy link
Contributor

lewisjared commented Oct 14, 2025

I still haven't had the chance to run this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lewisjared lewisjared lewisjared left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

REF V1
Status: In progress

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Improve support for adding parent experiment data

2 participants

@bouweandela @lewisjared