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.

Sign up for GitHub

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

Jump to bottom

Restructure SEAICETHERMO components and make sea ice grid comps built as DSOs #868

Draft
wants to merge 5 commits into
base: develop
Choose a base branch
from
Draft

Restructure SEAICETHERMO components and make sea ice grid comps built as DSOs #868

wants to merge 5 commits into from

Conversation

zhaobin74
Copy link
Contributor

This PR achieves:

Passed 0-diff for both CICE4 and CICE6 coupled tests.

@zhaobin74 zhaobin74 added the 0 diff The changes in this pull request have verified to be zero-diff with the target branch. label Dec 6, 2023
@zhaobin74 zhaobin74 requested review from a team as code owners December 6, 2023 14:28
tclune
tclune approved these changes Dec 6, 2023
View reviewed changes
Copy link
Collaborator

@tclune tclune left a comment

Choose a reason for hiding this comment

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

Glad to see this happening.

@sanAkel sanAkel added the Contingent - DNA These changes are contingent on other PRs (DNA=do not approve) label Dec 7, 2023
@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 7, 2023

This PR is being prevented from merging because you have added one of our blocking labels: Contingent - DNA, Needs Lead Approval, Contingent -- Do Not Approve. You'll need to remove it before this PR can be merged.

@sanAkel
Copy link
Contributor

sanAkel commented Dec 7, 2023 •
edited
Loading

While I don't have the time to these test changes, on behalf of @GEOS-ESM/seaice-team, I would like to make this contingent on a successfully run on macOS- given the troubles we have had with DSO on that OS.

Now that @zhaobin74, you have disentangled these dependencies, an alternative is the build option like for ocean model,, in which case, we have no contingencies. Yay!

sanAkel
sanAkel reviewed Dec 7, 2023
View reviewed changes
Copy link
Contributor

@sanAkel sanAkel left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Please test on a mac and also with data sea ice

or

provide build time options, defaulting to CICE4.

@zhaobin74
Copy link
Contributor Author

Please test on a mac and also with data sea ice

Data sea ice also passed 0-diff.
Unfortunately I don't have experience and bandwidth to test this on a mac. Unless someone could help, I have to draft this PR.

or

provide build time options, defaulting to CICE4.

@zhaobin74 zhaobin74 added the help wanted Extra attention is needed label Dec 7, 2023
@sanAkel sanAkel marked this pull request as draft December 7, 2023 18:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sanAkel sanAkel sanAkel left review comments

@tclune tclune tclune approved these changes

@yvikhlya yvikhlya yvikhlya approved these changes

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
0 diff The changes in this pull request have verified to be zero-diff with the target branch. Contingent - DNA These changes are contingent on other PRs (DNA=do not approve) help wanted Extra attention is needed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@zhaobin74 @sanAkel @yvikhlya @tclune