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

Update Icepack to 4c42a82, non bit-for-bit changes #499

Merged
merged 2 commits into from
Aug 8, 2020

Conversation

apcraig
Copy link
Contributor

@apcraig apcraig commented Aug 5, 2020

PR checklist

Incorporates two important bug fixes from Icepack that are not bit-for-bit,

Also removes two tests that are failing and will not fix. With recent updates to NETCDF implementation, configurations with iobinary that have bathymetry on will fail (as they should). These tests were removed.

Copy link
Contributor

@dabail10 dabail10 left a comment

Choose a reason for hiding this comment

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

Do we want to bring both in at once? Maybe since we are not tagging them, it doesn't matter?

@apcraig
Copy link
Contributor Author

apcraig commented Aug 6, 2020

I'm happy to bring them in separately, although I'm not sure it provides much advantage. We could demonstrate how each Icepack change separately changes answers, but the Icepack PRs largely do that already as they were tested in CICE independently there. If others think we should separate, just let me know, and I'll take care of it.

@dabail10
Copy link
Contributor

dabail10 commented Aug 6, 2020

Point taken. I have tested the answer changes in standalone Icepack as well as in CICE individually. Both pass the QC control tests. So, I am fine with these going into CICE together if others are.

@dabail10 dabail10 self-requested a review August 6, 2020 15:27
@apcraig apcraig merged commit 12d16ed into CICE-Consortium:master Aug 8, 2020
Copy link
Contributor

@eclare108213 eclare108213 left a comment

Choose a reason for hiding this comment

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

I'm fine with merging all of this together.

@apcraig apcraig deleted the ipup06 branch August 17, 2022 21:00
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