-
Notifications
You must be signed in to change notification settings - Fork 53
Set under/over indices for sea ice conc and thickness #1083
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
Conversation
|
Addresses #1082 |
|
It's better to put the "Addresses ..." comment in the description of the PR because then it closes the issue when the PR gets merged. Normal comments unfortunately don't do that. |
b6405b7 to
d2c2ee9
Compare
TestingI ran a test from the test suite on this branch and I now see the expected over/under on thickness and concentration, e.g.: I'm rerunning the rest of the suite now. |
|
The main vs. ctrl run failed with out the fix in #1084. Trying again with it... |
|
Okay, I have a working main vs. ctrl: |
|
@cbegeman and @darincomeau, could you give this a very quick review just by inspection and from whatever of my test suite results you want to look at? I don't want to waste a lot of your time but I also think it would be worth making sure this doesn't seem like it'll have unexpected side effects or something. |
cbegeman
left a comment
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.
@xylar Looks good to me. Thanks!
darincomeau
left a comment
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.
@xylar looks good to me as well - thanks!
|
Great, thanks! |

This merge sets

extendtoboth, rather thanneitherfor difference plots of sea-ice concentration and thickness. The previous value ofneitherled to plots like this one:In this PR have slightly updated the indices into the colormap and explicitly added indices for the unser/over (not necessary but there for clarity).
Checklist
Testingcomment in the PR documents testing used to verify the changesFixes #1082