-
Notifications
You must be signed in to change notification settings - Fork 10
Split regional historical recipes into model and reference runs #460
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.
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
Split regional historical recipes into model and reference runs #460
Conversation
Codecov Report❌ Patch coverage is
🚀 New features to boost your workflow:
|
563b49b to
79c96fd
Compare
bbdb54b to
fda40b4
Compare
ad31ec8 to
a8fd54b
Compare
| if not isinstance(requirement, DataRequirement): | ||
| raise TypeError(f"Expected a DataRequirement, got {type(requirement)}") | ||
| if requirement.source_type not in data_catalog: | ||
| raise InvalidDiagnosticException( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test cases fail with this exception because they are run with a catalog that only contains CMIP6 or obs4MIPs data, not both, so I downgraded this to a debug message.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is fine. I think this isn't possible for a user using the CLI
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It happens when I run ref test-cases run --provider esmvaltool
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh yup I meant a solve, but I also bumped into this in #510 so definitely a bug
| if not isinstance(requirement, DataRequirement): | ||
| raise TypeError(f"Expected a DataRequirement, got {type(requirement)}") | ||
| if requirement.source_type not in data_catalog: | ||
| raise InvalidDiagnosticException( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is fine. I think this isn't possible for a user using the CLI
Description
Split regional historical recipes into model and reference runs and preprocess all regions in one go. This avoids loading the data multiple times and rerunning relatively expensive computations to compute the reference for each model. The downside is that the plots created by ESMValTool now will only contain the model or reference, so it is up to the REF portal to create a series figure where both are visible.
Checklist
Please confirm that this pull request has done the following:
changelog/