-
Notifications
You must be signed in to change notification settings - Fork 52
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 OHC anomaly to timeSeriesOceanRegions
#1034
Conversation
An example result from the test suite: I will request reviews as soon as I have run this on a SORRM v2.1 simulation (with the southern hemisphere geometric feature MPAS-Dev/geometric_features#205 included). |
5f1d193
to
e95af32
Compare
I just noticed some glitches in the figure above. I think I've fixed them an will replace it if so. |
Somewhat better but the title is still a mess. |
e680a6c
to
142c290
Compare
Here's the SORRM v2.1 ensemble mean analysis over the historical: |
@cbegeman and @darincomeau, For the v2.1 paper, several details and associated questions:
|
I won't merge this until we get MPAS-Dev/geometric_features#205 merge, do a release of geometric_features, and update the constraint on geometric_features here. |
6ee97df
to
8f607dd
Compare
Further testingI reran the test suite after updating to geometric_features 1.6.0. I needed to fix the name of Greenland Regions --> ISMIP6 Greenland Regions for things to work. With this fix, all analysis ran successfully as seen here: However, there was a glitch in the anomaly computation for the control run. Fixing this... |
@xylar Great! Looking through the code, I don't see anything to change. Would you like me to do some testing, perhaps generating the v2.1 SORRM Southern Ocean OHC anomaly plot so it could be useful for the paper? |
@cbegeman, it certainly would be fine if you want to test this on SORRM v2.1 I've done that on an earlier version above but it wouldn't hurt to run it again. You can use my config file in the link above as a starting point. |
Here's the link to the main page with the config file. |
c4ec46b
to
8754132
Compare
Rebased, but this includes #1040 for now. |
Rather than defining the full list of dictionaries for each region, define one list of dict of available variables and then just a list of variables for each region.
The region name is getting completely cut off in some cases.
8754132
to
29eead2
Compare
@cbegeman, if you don't have time to run new tests, I think that's fine. I'm pretty sure my previous tests with the SORRM v2.1 ensemble mean over the historical are sufficient. I wouldn't mind getting this merged by the end of this week. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @cbegeman! |
Checklist
Testing
comment in the PR documents testing used to verify the changes