Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add new 0.66 degree MOM6 grid into make_bcs #712
Add new 0.66 degree MOM6 grid into make_bcs #712
Changes from 5 commits
7398a3d
377221d
2ac19b0
077bb4d
5b03ac9
aee586c
69d1e8d
19d12f8
e4aa8dc
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
@sdrabenh, I suspect reason for failure(s) is that it cannot read there new files. Right @mathomp4 ?
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.
Now, I intend to check it all myself. I have queued it, but have no ETA when I'll get to it.
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.
@sanAkel sorry just saw your comment. That is what I suspect
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.
Boundary Conditions package would have failed @sanAkel if files weren't there.
So files were at location when I was running and are still there.
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.
@sanAkel @sdrabenh I don't think it's that. It's dying in run so it gets through the scripting.
My first guess is that the CI does a
checkout-if-exists
step and so it's not just picking up this branch but also theyv/feature/mom6-0.66deg
in GEOS_OceanGridComp as seen in GEOS-ESM/GEOS_OceanGridComp#21That PR from @yvikhlya is based on
develop
in the Ocean GC repo and in there line 653 (which is where the model is dying) is crapping out on:If that line requires, say, a new mom6 tag or something, it's possible the CI is just "unhappy" until the latest Ocean/MOM6 combo is brought into GEOSgcm fixture itself.
So this might be a case of we have to superpower merge it in because of CI issues.
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.
@sanAkel It's just really a consequence of the CI scripting trying to be smart. This seems "safe" to pull in because, as @gmao-rreichle says below, it's touching a script my CI does not care about. But the
yv/feature/mom6-0.66deg
branch in Ocean GC doesn't quite run without some change elsewhere.If you, @sdrabenh, @biljanaorescanin, and @gmao-rreichle are fine with this, I can power it in with admin powers.
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.
It touches only several configs. How can this affect the build?
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.
I'm generally ok with the changes but we MUST coordinate this PR with #707. At this moment, #707 is ready, and I assume @sdrabenh will merge it first. Then we have to modify this PR in response. So please refrain from using superpowers unless we have this sorted, so we're not inadvertently merging a conflicting set of PRs.
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.
I go with @gmao-rreichle - his word be the final one!
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.
@gmao-rreichle Please put a DNA label