Skip to content

Conversation

@xylar
Copy link
Collaborator

@xylar xylar commented Apr 7, 2025

This merge sets extend to both, rather than neither for difference plots of sea-ice concentration and thickness. The previous value of neither led to plots like this one:
image

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

  • User's Guide has been updated
  • Testing comment in the PR documents testing used to verify the changes

Fixes #1082

@xylar xylar added the bug label Apr 7, 2025
@xylar xylar requested a review from cbegeman April 7, 2025 16:00
@xylar xylar self-assigned this Apr 7, 2025
@cbegeman
Copy link
Collaborator

cbegeman commented Apr 7, 2025

Addresses #1082

@xylar xylar requested a review from darincomeau April 7, 2025 16:29
@xylar
Copy link
Collaborator Author

xylar commented Apr 7, 2025

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.

@xylar xylar force-pushed the fix-sic-diff-over-under branch from b6405b7 to d2c2ee9 Compare April 7, 2025 16:59
@xylar
Copy link
Collaborator Author

xylar commented Apr 7, 2025

Testing

I ran a test from the test suite on this branch and I now see the expected over/under on thickness and concentration, e.g.:
https://web.lcrc.anl.gov/public/e3sm/diagnostic_output/ac.xasay-davis/analysis_testing/chrysalis/fix-sic-diff-over-under/main_py3.11/sea_ice/

image

I'm rerunning the rest of the suite now.

@xylar
Copy link
Collaborator Author

xylar commented Apr 7, 2025

The main vs. ctrl run failed with out the fix in #1084. Trying again with it...

@xylar
Copy link
Collaborator Author

xylar commented Apr 7, 2025

@xylar
Copy link
Collaborator Author

xylar commented Apr 7, 2025

@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.

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.

@xylar Looks good to me. Thanks!

Copy link
Contributor

@darincomeau darincomeau left a 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!

@xylar
Copy link
Collaborator Author

xylar commented Apr 7, 2025

Great, thanks!

@xylar xylar merged commit 3cf93a9 into MPAS-Dev:develop Apr 7, 2025
5 checks passed
@xylar xylar deleted the fix-sic-diff-over-under branch April 7, 2025 22:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Sea ice colormap set_under color

3 participants