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

Fix isomip_plus streamfunction computation #548

Merged
merged 2 commits into from
Mar 2, 2023
Merged

Fix isomip_plus streamfunction computation #548

merged 2 commits into from
Mar 2, 2023

Conversation

xylar
Copy link
Collaborator

@xylar xylar commented Mar 1, 2023 •
edited
Loading

The fix involves replacing NaNs in transport with zeros, so that the resulting sums are not themselves contaminated with NaNs.

This merge also contains some linting-related clean up.

Checklist

  • Document (in a comment titled Testing in this PR) any testing that was used to verify the changes

xylar added 2 commits March 1, 2023 10:10
@xylar
Lint isomip_plus streamfunction
30a2e99
@xylar
Fill NaNs in transport with zeros
0248a4a 
This is necessary when computing streamfunctions so the resulting
sums are not themselves contaminated with NaNs.
@xylar xylar self-assigned this Mar 1, 2023
@xylar xylar added bug Something isn't working ocean labels Mar 1, 2023
@xylar xylar requested a review from cbegeman March 1, 2023 10:30
@xylar
Copy link
Collaborator Author

xylar commented Mar 1, 2023

Testing

I was able to run the 5 km zstar Ocean test case (including simulation, streamfunction and viz steps) with this fix, whereas I got the error reported in #471 (review) without it.

See:

/lcrc/group/e3sm/ac.xylar/compass_1.2/chrysalis/test_20230301/ocean0_5km_fix_osf2/ocean/isomip_plus/5km/z-star/Ocean0

cbegeman
cbegeman approved these changes Mar 2, 2023
View reviewed changes
Copy link
Collaborator

@cbegeman cbegeman left a comment

Choose a reason for hiding this comment

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

I tested 5km/z-star/Ocean0 and 5km/sigma/Ocean0 on chrysalis and they both looked great! I'm glad the fix was so simple!

@xylar xylar merged commit 3a4befc into MPAS-Dev:main Mar 2, 2023
@xylar xylar deleted the fix_isomip_plus_osf branch March 2, 2023 20:50
@xylar
Copy link
Collaborator Author

xylar commented Mar 2, 2023

Great, thanks @cbegeman!

@xylar xylar mentioned this pull request Mar 2, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cbegeman cbegeman cbegeman approved these changes

Assignees

@xylar xylar

Labels
bug Something isn't working ocean
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@xylar @cbegeman