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.

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

Clean up RPE analysis step and framework #123

Merged
merged 1 commit into from
Sep 27, 2023

Conversation

xylar
Copy link
Collaborator

@xylar xylar commented Sep 27, 2023

Rather than using some iffy custom plotting techniques, this uses the framework plot_horiz_field() function.

This merge also fix a bug in the placement of nu being added to model config options. Before this fix, the forward.yaml options were overwriting the nu value so all RPE runs used the same viscosity. This problem was introduced in the initial port of baroclinic channel (875e7fe) but apparently after initial testing showed that it worked as expected.

Checklist

  • Testing comment in the PR documents testing used to verify the changes

Fix the place where nu is added to model config options.
@xylar
Copy link
Collaborator Author

xylar commented Sep 27, 2023

Testing

I successfully ran the 10 km RPE test on Chrysalis. The updated output is:
sections_baroclinic_channel_10 0
rpe_t

For reference, old output from #27 was:
image
image

@xylar xylar requested a review from cbegeman September 27, 2023 11:18
@xylar
Copy link
Collaborator Author

xylar commented Sep 27, 2023

@cbegeman, a quick review by inspection would be appreciated.

It might be nice to add the ability to share y labels and have a single colorbar like in the original plot, but that isn't a high priority for me right now.

Comment on lines +38 to +40
xEdge = ds_mesh.xEdge
yEdge = ds_mesh.yEdge
areaCell = ds_mesh.areaCell
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This change is in the spirit of not assuming that mesh variables are in the initial condition.

@xylar xylar added bug Something isn't working clean-up ocean Related to ocean tests or analysis labels Sep 27, 2023
@xylar xylar self-assigned this Sep 27, 2023
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.

Looks good by visual inspection. Thanks for the clean-up and fix!

@xylar xylar merged commit 0cb07fc into E3SM-Project:main Sep 27, 2023
5 checks passed
@xylar xylar deleted the clean-up-rpe-analysis branch September 27, 2023 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working clean-up ocean Related to ocean tests or analysis
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants