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

Allow patches to be reused in plot_horiz_field() #146

Merged
merged 3 commits into from
Nov 2, 2023

Conversation

xylar
Copy link
Collaborator

@xylar xylar commented Oct 31, 2023

This merge returns patches and patch_mask variables from plot_horiz_field() so they can be reused in subsequent calls. This should save a lot of time in tests that plot many fields. Corresponding input arguments have been added to take patches and patch_mask from previous plots on the same mesh.

Note: you need need to keep patches for cells and edges separate.

This merge also removes some ocean-specific variable and dimension names. This because the function lives in the top-level framework so it should work for any component. Instead of providing maxLevelCell in ds, the function needs to be called with a cell_mask argument that should typcially be cell_mask = ds_init.maxLevelCell >= 1 (though it could be another field such as landIceFloatingMaks if appropriate).

All steps that call plot_horiz_field() have been updated to supply the cell_mask argument or to use patches and patch_mask from a previous call.

Checklist

  • Developer's Guide has been updated
  • Documentation has been built locally and changes look as expected
  • Testing comment in the PR documents testing used to verify the changes

@xylar xylar self-assigned this Oct 31, 2023
@xylar xylar changed the title Reuse patches in planar horiz Allow patches to be reused in plot_horiz_field() Oct 31, 2023
@xylar xylar added enhancement New feature or request clean-up framework Changes relating to the polaris framework as opposed to individual tests or analysis ocean Related to ocean tests or analysis labels Oct 31, 2023
@xylar xylar force-pushed the reuse-patches-in-planar-horiz branch from d3a1098 to 4c5570a Compare October 31, 2023 12:44
@xylar xylar requested a review from cbegeman October 31, 2023 12:45
@xylar xylar force-pushed the reuse-patches-in-planar-horiz branch from 97fe053 to a44f53b Compare October 31, 2023 13:33
@xylar
Copy link
Collaborator Author

xylar commented Oct 31, 2023

Testing

I ran all planar tasks (except 4km and 1km baroclinic channel RPE) on Chrysalis. I included the viz steps for the inertial gravity wave and manufactured solutions tasks. All images look as expected, e.g.:
comparison

@xylar
Copy link
Collaborator Author

xylar commented Oct 31, 2023

Further testing

This has sped up the init step of ISOMIP+ tasks at 1 km resolution (in preliminary testing) by more than a factor of 3 from ~32 minutes to ~9 minutes.

@xylar
Copy link
Collaborator Author

xylar commented Nov 1, 2023

Oooh! The speed-up is far better than I thought! I messed up the culling in an unrelated change to this one. After fixing the culling, the init step for ISOMIP+ at 1 km resolution takes less than a minute (even with smoothing over 2 cells)!!!

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.

This looks great! I have tested inertial_gravity_wave on chrys with intel, openmpi since that is one of the tests that reuses patches. The viz looks the same as before.

@xylar
Copy link
Collaborator Author

xylar commented Nov 2, 2023

@cbegeman, thanks for your review and testing! This will be great to have in!

@xylar xylar merged commit 0527fc5 into E3SM-Project:main Nov 2, 2023
@xylar xylar deleted the reuse-patches-in-planar-horiz branch November 2, 2023 09:27
'from a previous call to plot_horiz_field()')
else:
if cell_mask is None:
cell_mask = np.ones_like(field, type='bool')
Copy link
Collaborator

Choose a reason for hiding this comment

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

@xylar This needs to be dtype. Do you want me to open a separate bugfix PR or do you want to combine this with #144?

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 should be a separate PR. I'm not sure how soon #144 will be ready to go in. Sorry about that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clean-up enhancement New feature or request framework Changes relating to the polaris framework as opposed to individual tests or analysis ocean Related to ocean tests or analysis
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants