PMP enso #273
Merged
PMP enso #273
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lewisjared
reviewed
May 7, 2025
packages/climate-ref-pmp/src/climate_ref_pmp/diagnostics/enso.py
Outdated
Show resolved
Hide resolved
packages/climate-ref-pmp/src/climate_ref_pmp/diagnostics/enso.py
Outdated
Show resolved
Hide resolved
lee1043
commented
May 9, 2025
packages/climate-ref-pmp/src/climate_ref_pmp/requirements/environment.yml
Show resolved
Hide resolved
* origin/main: (38 commits) Add changelog Bump sample data to v0.5.1 Add regression test data chore: Remove old migration for metric_value columns test: Ensure that run is correctly mocked chore: Fix last test chore: Update cmec metric bundles revert: Roll back changing the data requirements for the modes of variability chore: revert some changes to avoid to many changes in this PR test: Add tests for remove_dimensions test: Fix tests fix: Add additional facets to variability modes fix: Add additional facets to annual_cycle chore: Remove excess dimensions and replace with selectors chore: Remove excess dimensions and replace with selectors test: Add ilamb integration test feat: Add a prepend routine chore: Check that metric bundles have the correct dimensions chore: Reorder functions docs: Changelog ...
lewisjared
reviewed
May 9, 2025
packages/climate-ref-pmp/src/climate_ref_pmp/diagnostics/enso.py
Outdated
Show resolved
Hide resolved
lewisjared
reviewed
May 9, 2025
packages/climate-ref-pmp/src/climate_ref_pmp/diagnostics/enso.py
Outdated
Show resolved
Hide resolved
|
@lee1043 This doesn't work for me. The regridding is always failing. I've used the decimated and complete files with the same errors. Have you seen this before? PET0.ESMF_LogFile |
|
@lewisjared thanks for testing. The error looks new to me. Let me see if I can reproduce and pin it down. |
Thanks for offering @lee1043! I would be keen to stick with the plan for now, so as not to disappoint the folks at ACCESS-NRI who worked hard to get their ENSO diagnostics ready for inclusion in the REF in time. |
|
@lee1043 Did you get a chance to try and reproduce my error? If you couldn't could you note down when steps and data you used to get this to work? |
|
@lewisjared I added couple commits that resolves the error you had. Could you test when you get a chance? I also noticed that there is a conflict from pyproject.toml, can you help addressing it? I will work on setting up the updated ref data pool. |
* origin/main: (60 commits) chore: Use sha1sums for hashes chore: Changelog chore: Add additional dataset to ilamb/iomb Bump version: 0.5.4 → 0.5.5 chore: Move to the newer version of development dependencies chore: Unify pyproject classifiers docs: Changelog chore: Print more outputs docs: More basic documentation Recreate TCR regression test output chore: Cleanup aft regression test chore: regenerate regression output Add changelog Remove superfluous recipe keys in TCR diagnostic and rename diag Add changelog Avoid using 'cmip6' in diag name Add tests Remove superfluous recipe keys in ECS diagnostic chore: Fix integration tests chore: Add ilamb prefix everywhere ...
|
With the updated reference dataset below, I think this PR is ready to be merged. It looks like the CI/tests is failing for python 3.13, but PMP is yet to support python 3.13 so I think it is expected. @lewisjared do you think it is okay to merge this PR? |
|
This still needs regression tests and I haven't been able to verify that it works locally yet so it isn't ready to merge. The failing test was due to the lack of coverage. I think that the decimated data breaks the regridding so we might need to ignore this in the integration tests. If it isn't easy to test then we need some locally generated regression output so that we can check that the results parsing works (that will help the coverage). If you can zip up and send me some generated output that would be useful. I don't want to merge this until we have either the right reference data or some regression output from the test and an understanding of what data is needed that isn't in the tests. |
lewisjared
reviewed
May 23, 2025
|
I've noticed that some of these are newer versions of the other reference datasets you mentioned above or they have different hashes. Which ones should be used? |
Datasets in the latest comment are the sole set needed for PMP ENSO. Once you have them and a model data with right variables, it will enable running local test. I will also zip up the output files that can be used for regression tests. |
* origin/main: chore: Fix edge case of an empty catalog chore: Changelog feat: Exclude all but the latest version chore: Update count of datasets chore: Add additional obs data to the registry test: coverage test: Fix docs: Changelog feat: split file logging by process feat: Write out verbose log files for debugging chore: Changelog chore: Disable marking CV failures as failed executions test: Update tests chore: Enable marking executions as failed if they don't have the correct cv chore: Regenerate regression tests
Codecov Report❌ Patch coverage is
🚀 New features to boost your workflow:
|
lewisjared
approved these changes
May 26, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Checklist
Please confirm that this pull request has done the following:
changelog/