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

Isotope #275

Closed
wants to merge 29 commits into from
Closed

Isotope #275

wants to merge 29 commits into from

Conversation

dabail10
Copy link
Contributor

@dabail10 dabail10 commented Sep 5, 2019

#91 PR checklist

@apcraig
Copy link
Contributor

apcraig commented Sep 5, 2019

One issue is that it looks like at least one (maybe more) interface (icepack_atm_boundary) is modified which will break CICE until CICE is updated and tested. If we push this to master, we cannot test any icepack changes in CICE and icepack master will be broken with respect to CICE. I think my preference would be that CICE modifications be carried out before we merge this PR. This will ensure that the new icepack mods are working in CICE and it will not block other changes to icepack moving forward.

This is one area where the current icepack interfaces are not that great, extensibility. And by the way, I still don't know how/if we should proceed with any icepack interface refactor at this point. But that's a separate issue.

@dabail10
Copy link
Contributor Author

dabail10 commented Sep 5, 2019

Excellent point. I will modify the above that this requires a CICE PR as well.

@apcraig
Copy link
Contributor

apcraig commented Sep 17, 2019

You can ignore this comment, just testing something.

@apcraig
Copy link
Contributor

apcraig commented Feb 19, 2020

@dabail10. Even if this is not quite ready for usage, I still have the same question as in September. If we merge this now/soon, will be unable to update CICE with new Icepacks due to interface changes? Can we have a CICE PR that addresses this issue? We do not want to make the Icepack master unusuable in CICE for any extended period of time. It's fine if it's a couple days and we have a plan to get past it. But I worry that's not the case here.

@dabail10
Copy link
Contributor Author

I am doing extensive testing with both Icepack and CICE updated to master. This should be ready soon.

@apcraig apcraig mentioned this pull request Apr 3, 2020
16 tasks
@eclare108213
Copy link
Contributor

This PR is being replaced by #307, so closing it now.

@apcraig apcraig mentioned this pull request Apr 10, 2020
@dabail10 dabail10 deleted the isotope branch October 4, 2022 17:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants