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

Use MPAS-Tools for great circle distance in cosine bell tests #127

Merged
merged 4 commits into from
Oct 5, 2023
Merged

Use MPAS-Tools for great circle distance in cosine bell tests #127

merged 4 commits into from
Oct 5, 2023

Conversation

cbegeman
Copy link
Collaborator

@cbegeman cbegeman commented Oct 2, 2023 •
edited
Loading

This PR replaces the great circle distance computation in cosine_bell with that in MPAS-Tools. It is based on #126.

Checklist

  • API documentation in the Developer's Guide (api.md) has any new or modified class, method and/or functions listed
  • Documentation has been built locally and changes look as expected
  • Testing comment in the PR documents testing used to verify the changes
    User's Guide has been updated
    Developer's Guide has been updated
    New tests have been added to a test suite

@cbegeman
Copy link
Collaborator Author

cbegeman commented Oct 2, 2023

Testing

The icos cosine_bell case has been run and it has machine-precision level diffs with main due to the great circle computation.

@cbegeman cbegeman requested a review from xylar October 2, 2023 21:49
@cbegeman cbegeman mentioned this pull request Oct 2, 2023
Merged
7 tasks
@cbegeman cbegeman force-pushed the ocn-standardize-great-circle-distance branch 3 times, most recently from d4522b3 to 0934d24 Compare October 3, 2023 22:35
@cbegeman
Copy link
Collaborator Author

cbegeman commented Oct 3, 2023

@xylar This is now rebased on #126 and ready for review, though you can wait until after that one is merged.

@xylar xylar mentioned this pull request Oct 4, 2023
Merged
5 tasks
@cbegeman cbegeman force-pushed the ocn-standardize-great-circle-distance branch from 0934d24 to 8c2df45 Compare October 4, 2023 22:09
@xylar
Copy link
Collaborator

xylar commented Oct 5, 2023

@cbegeman, I'm happy to review this once it gets rebased onto main.

@cbegeman
Copy link
Collaborator Author

cbegeman commented Oct 5, 2023

@xylar It's rebased. Thanks for reviewing!

@cbegeman cbegeman force-pushed the ocn-standardize-great-circle-distance branch from 8c2df45 to 9fe4332 Compare October 5, 2023 14:58
@xylar xylar assigned xylar and cbegeman and unassigned xylar Oct 5, 2023
@xylar xylar added clean-up ocean Related to ocean tests or analysis enhancement New feature or request and removed clean-up labels Oct 5, 2023
xylar
xylar approved these changes Oct 5, 2023
View reviewed changes
Copy link
Collaborator

@xylar xylar left a comment

Choose a reason for hiding this comment

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

Looks great! I ran the icos cosine bell with viz and looked at the output. I didn't compare with the baseline so I'll trust your testing on that. But this is more correct regardless.

@xylar
Copy link
Collaborator

xylar commented Oct 5, 2023

Please merge when you're reandy.

@cbegeman cbegeman merged commit c737933 into E3SM-Project:main Oct 5, 2023
@cbegeman
Copy link
Collaborator Author

cbegeman commented Oct 5, 2023

@xylar Thank you for reviewing!

@xylar xylar deleted the ocn-standardize-great-circle-distance branch October 5, 2023 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xylar xylar xylar approved these changes

Assignees

@cbegeman cbegeman

Labels
enhancement New feature or request ocean Related to ocean tests or analysis
Projects
None yet
Milestone
No milestone
Development

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