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

Add Southern Hemisphere region #205

Merged
merged 4 commits into from
Nov 6, 2024

Conversation

cbegeman
Copy link
Collaborator

@cbegeman cbegeman commented Sep 2, 2024

Add a region for the Southern Hemisphere

@cbegeman
Copy link
Collaborator Author

cbegeman commented Sep 2, 2024

@xylar Can you take a look at this and let me know if you see any issues? I'm having issues using it with MPAS-Analysis but I think something might be off with my editable environment because when I use a modified pytest to check these features they seem fine.

@cbegeman cbegeman requested a review from xylar September 2, 2024 23:07
@cbegeman cbegeman self-assigned this Sep 2, 2024
@xylar
Copy link
Collaborator

xylar commented Sep 3, 2024

@cbegeman, yes, it looks fine to me.

@cbegeman
Copy link
Collaborator Author

cbegeman commented Sep 3, 2024

@xylar Would you be willing to give this a try in MPAS-Analysis? I was changing the regions option in timeSeriesOHCAnomaly but I don't know if I'm getting the syntax right on the region name that corresponds to Southern Hemisphere. It seemed to fail with or without an underscore.

@xylar
Copy link
Collaborator

xylar commented Sep 3, 2024

@cbegeman, sure, I can give it a try tomorrow.

@xylar
Copy link
Collaborator

xylar commented Sep 4, 2024

@cbegeman, I see the problem. timeSeriesOHCAnomaly uses output from the online Regional Stats analysis member. This is really unfortunate because those regions are hard-coded.

We could presumably write a post-processed version of OHC that could take a region from a mask but that doesn't yet exist. Or alternatively wouldn't OHC be trivial to calculate from mean temperature in a region (using timeSeriesOceanRegions), volume of the region, heat capacity, etc.? Or am I missing something?

@milenaveneziani
Copy link
Collaborator

@xylar: you are not missing anything. I always thought that OHC analysis tasks could be done in post-process mode (along with regional statistics, to be honest). In fact, I believe that the other OHC task, the climatological maps, is done using the layer-weighted averaged temperature.

@cbegeman
Copy link
Collaborator Author

cbegeman commented Sep 4, 2024

Thanks @xylar and @milenaveneziani. It seems like post-processing OHC makes a lot of sense and could allow the AM to be removed in OMEGA. I'll implement it in MPAS-Analysis. This work was motivated by being able to compare our SORRM OHC with published values from CMIP6 models.

'date': '20200621',
'date': '20240830',
Copy link
Collaborator

Choose a reason for hiding this comment

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

@cbegeman, I updated the date stamp for Ocean Basins. I hope you don't mind.

This is needed so new region masks get created in MPAS-Analysis and Compass that include Southern Hemisphere.

@cbegeman
Copy link
Collaborator Author

cbegeman commented Nov 5, 2024

@xylar Does this look ready for review to you? Seems like you successfully used it in your MPAS-A PR

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.

This looks good to me and, yes, I was able to test it in MPAS-Analysis. It successfully produced ocean heat content integrated over the region:
MPAS-Dev/MPAS-Analysis#1034 (comment)

@xylar
Copy link
Collaborator

xylar commented Nov 5, 2024

@cbegeman, remind me that we'll want to cache some mask files for typical meshes with these new regions to save MPAS-Analysis from generating them every time it runs. This would be once this is being used in MPAS-Analysis, of course.

@cbegeman cbegeman marked this pull request as ready for review November 5, 2024 20:07
@cbegeman
Copy link
Collaborator Author

cbegeman commented Nov 5, 2024

Thanks @xylar for your testing. I'll merge now.

@cbegeman
Copy link
Collaborator Author

cbegeman commented Nov 5, 2024

@xylar Actually, I don't have permissions to merge so you'll have to do it.

@xylar xylar assigned xylar and unassigned cbegeman Nov 6, 2024
@xylar
Copy link
Collaborator

xylar commented Nov 6, 2024

@cbegeman, for the future, I added you as a maintainer. But happy to merge this.

@xylar xylar merged commit ac7658c into MPAS-Dev:main Nov 6, 2024
4 checks passed
@cbegeman
Copy link
Collaborator Author

cbegeman commented Nov 6, 2024

@xylar Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants