Skip to content
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

Update MoV code to use xCDAT #1020

Merged
merged 101 commits into from
May 2, 2024
Merged

Update MoV code to use xCDAT #1020

merged 101 commits into from
May 2, 2024

Conversation

lee1043
Copy link
Contributor

@lee1043 lee1043 commented Jan 11, 2024

This is a draft PR, opened for tracking changes.

lee1043 and others added 24 commits January 8, 2024 12:44
@lee1043 lee1043 self-assigned this Jan 25, 2024
@lee1043 lee1043 added this to the 3.3 milestone Jan 25, 2024
@lee1043
Copy link
Contributor Author

lee1043 commented Apr 22, 2024 •
edited
Loading

Bug in centered RMSE calcaulation (left) is resolved (right).

Screenshot 2024-04-22 at 2 04 03 PM

More results: PMP_MoV_xCDAT_rigor_test_20240422.pdf

@lee1043
Copy link
Contributor Author

lee1043 commented Apr 22, 2024 •
edited
Loading

Progress note: next component that is needed is the custom season capability

@lee1043 lee1043 marked this pull request as ready for review April 24, 2024 22:32
@lee1043
Merge branch 'feature/1012_lee1043_stats-MoV_xcdat' of github.com:PCM…
6a414c6 
…DI/pcmdi_metrics into feature/1012_lee1043_stats-MoV_xcdat
@lee1043 lee1043 requested a review from acordonez April 24, 2024 22:33
@lee1043
Copy link
Contributor Author

lee1043 commented Apr 24, 2024

@acordonez when you have a chance could you please check if the demo4 notebook is working fine with this PR?

@acordonez
Copy link
Collaborator

@lee1043 The demo notebook ran successfully. Is it correct that some figures were removed from Section 3.3.2?

@lee1043
Copy link
Contributor Author

lee1043 commented Apr 26, 2024

@lee1043 The demo notebook ran successfully. Is it correct that some figures were removed from Section 3.3.2?

@acordonez thanks for checking. Yes, that is correct. For AMO I removed monthly example and kept only yearly example which likely be more interest.

@lee1043
Copy link
Contributor Author

lee1043 commented Apr 26, 2024 •
edited
Loading

@acordonez thanks for reviewing. I also asked Alex (LANL) and Shiheng (LLNL) for testing this code for their analysis. I am holding merge for their result.

@lee1043
Copy link
Contributor Author

lee1043 commented Apr 28, 2024 •
edited
Loading

Test results from Alex: test_AJ_LANL_20240427.pdf
"Everything worked without errors. The results look the same, although they are not identical (the 3rd NAO EOF came out to have a negative center of activity with my old code and a positive one with the new code version. Patterns look the same though."

My comment:
Seeing consistent results between the old and new codes is great, although the sign reversion of the EOF3. Unless that reversion would significantly impact your analysis, I don’t worry too much about that as the EOF signs are determined arbitrarily even with a very subtle difference in the input. I suspect the change in building block (CDMS to xCDAT) may have resulted difference in higher decimal numbers. Relevant discussion can be found here.

@lee1043 lee1043 linked an issue Apr 28, 2024 that may be closed by this pull request
Convert MoV code to use xcdat #1086
Closed
lee1043
lee1043 commented May 1, 2024
View reviewed changes
pcmdi_metrics/variability_mode/variability_modes_driver.py
run = re.split("[._]", (model_path.split("/")[-1]).split("."))[
run_in_modpath
]
run = re.split("[._]", model_path.split("/")[-1])[run_in_modpath]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Shiheng found bug in this line when file is named like “file_name_1.nc”. This fix should work well either cases like file is named like “file_name_1.nc” or “file.name.1.nc”

Copy link
Contributor Author

Choose a reason for hiding this comment

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

During the mean time, his comment was "I’ve checked the first EOF mode of PDO. The result looks good to me."

@lee1043 lee1043 merged commit 6d6b688 into main May 2, 2024
5 checks passed
@lee1043 lee1043 deleted the feature/1012_lee1043_stats-MoV_xcdat branch May 2, 2024 00:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@acordonez acordonez acordonez approved these changes

Assignees

@lee1043 lee1043

Labels
CRITICAL enhancement
Projects
None yet
Milestone
3.4
Development

Successfully merging this pull request may close these issues.

Convert MoV code to use xcdat
2 participants
@lee1043 @acordonez