Add new 0.66 degree MOM6 grid into make_bcs #712

yvikhlya · 2023-03-02T20:13:04Z

and return back old 360x210 MOM6 grid in case someone wants to run it with old land.

Sync develop into main

gmao-rreichle · 2023-03-02T22:05:27Z

Thanks, @yvikhlya. Looks ok to me but...

@biljanaorescanin should test the revised make_bcs script just to make sure it works, if you haven't done this already
cleanup and reorganization of files in ./Raster (make_bcs) #707 also touches the make_bcs script by moving it into a subdir as part of the cleanup. We need to be careful about merging the two PRs in the right order.
We are working on py versions of the make_bcs script, which are already on the develop branch and will need to be updated with the changes in the present PR. @weiyuan-jiang will need to take care of this.

Once item 1. is addressed, I'm happy to approve the PR. We'll then need to work with @sdrabenh to merge #707 and the present PR in the correct order (item 2.). We don't have to wait for item 3 since the py versions aren't ready for prime time anyway, but we have to remember to make the changes sooner rather than later.

sanAkel · 2023-03-03T14:44:19Z

@yvikhlya Will either add your river routing patch script to this PR or provide a link to it (by posting it elsewhere)?

Once you do it, I'll take the DNA label off- because without, nobody can reproduce the set up.

Thanks!

github-actions · 2023-03-03T14:44:29Z

Label error. Requires exactly 0 of: Contingent - DNA, Needs Lead Approval, Contingent -- Do Not Approve. Found: enhancement, Contingent - DNA, 0 diff structural

yvikhlya · 2023-03-03T15:49:26Z

I use this script
https://github.com/GEOS-ESM/GEOS_Util/blob/main/coupled_diagnostics/g5lib/src/reroute_runoff/reroute.py
to fix runoff, but I would strongly discourage to take it as any sort of fix to runoff problem, but rather as "do not use it again" hack to let us proceed with our tests asap. We still have to fix runoff issue a proper way (which is independent from this PR issue).

sanAkel · 2023-03-03T19:00:16Z

I use this script https://github.com/GEOS-ESM/GEOS_Util/blob/main/coupled_diagnostics/g5lib/src/reroute_runoff/reroute.py to fix runoff, but I would strongly discourage to take it as any sort of fix to runoff problem, but rather as "do not use it again" hack to let us proceed with our tests asap. We still have to fix runoff issue a proper way (which is independent from this PR issue).

Thank you @yvikhlya

And for the record the

We still have to fix runoff issue a proper way (which is independent from this PR issue).

is: #681

biljanaorescanin · 2023-03-06T13:29:01Z

@gmao-rreichle I've tested both options (T3MOM6 and T2MOM6) and they will produce whole set of boundary conditions. I presume science was tested by @sanAkel and @yvikhlya and resulting boundary conditions are what they expect to get.

sanAkel · 2023-03-06T13:40:48Z

@gmao-rreichle I've tested both options (T3MOM6 and T2MOM6) and they will produce whole set of boundary conditions. I presume science was tested by @sanAkel and @yvikhlya and resulting boundary conditions are what they expect to get.

No! I haven't yet tested.

@sdrabenh,
I recommend NOT to pull it until I finish ⬆️

GEOSagcm_GridComp/GEOSphysics_GridComp/GEOSsurface_GridComp/Utils/Raster/make_bcs

sdrabenh · 2023-03-06T15:55:12Z

Note, this PR is showing runtime failures with CircleCi. Has this PR been run or could this be an automated test issue? @mathomp4 is this a Ci data issue?

sanAkel · 2023-03-06T15:57:42Z

GEOSagcm_GridComp/GEOSphysics_GridComp/GEOSsurface_GridComp/Utils/Raster/make_bcs

 ln -s $MAKE_BCS_INPUT_DIR/ocean/MOM5/360x200 data/MOM5/360x200
 ln -s $MAKE_BCS_INPUT_DIR/ocean/MOM5/720x410 data/MOM5/720x410
 ln -s $MAKE_BCS_INPUT_DIR/ocean/MOM5/1440x1080 data/MOM5/1440x1080
 ln -s $MAKE_BCS_INPUT_DIR/ocean/MOM6/72x36 data/MOM6/72x36
+ln -s $MAKE_BCS_INPUT_DIR/ocean/MOM6/360x210 data/MOM6/360x210


#712 (comment)

@sdrabenh, I suspect reason for failure(s) is that it cannot read there new files. Right @mathomp4 ?

Now, I intend to check it all myself. I have queued it, but have no ETA when I'll get to it.

@sanAkel sorry just saw your comment. That is what I suspect

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.

@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 the yv/feature/mom6-0.66deg in GEOS_OceanGridComp as seen in GEOS-ESM/GEOS_OceanGridComp#21

That 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:

call MAPL_GetPointer(EXPORT, T_Freeze_e, 'T_Freeze', _RC)

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.

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

But the yv/feature/mom6-0.66deg branch in Ocean GC doesn't quite run without some change elsewhere.

It touches only several configs. How can this affect the build?

If you, @sdrabenh, @biljanaorescanin, and @gmao-rreichle are fine with this, I can power it in with admin powers.

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.

I go with @gmao-rreichle - his word be the final one!

If you, @sdrabenh, @biljanaorescanin, and @gmao-rreichle are fine with this, I can power it in with admin powers.

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.

@gmao-rreichle Please put a DNA label

gmao-rreichle · 2023-03-06T16:00:21Z

@sdrabenh: This PR only touches the "make_bcs" script, which isn't exercised in any of the CircleCI tests. Also, keep in mind that this PR will need updating after you merge #707, which moves "make_bcs" into a new location.

sdrabenh · 2023-03-06T16:39:31Z

Thanks @mathomp4 . If @gmao-rreichle @biljanaorescanin @sanAkel @yvikhlya are fine with this PR, I have no objection to @mathomp4 powering it through.

yvikhlya · 2023-03-06T16:42:22Z

ln -s $MAKE_BCS_INPUT_DIR/ocean/MOM6/360x210 data/MOM6/360x210

....

call MAPL_GetPointer(EXPORT, T_Freeze_e, 'T_Freeze', _RC)

None of the above was introduced by yv/feature/mom6-0.66deg PRs.

yvikhlya · 2023-03-06T16:43:47Z

Thanks @mathomp4 . If @gmao-rreichle @biljanaorescanin @sanAkel @yvikhlya are fine with this PR, I have no objection to @mathomp4 powering it through.

Still need to do 360x210 clean up. Wait for one more commit please.

Edit: Done.

github-actions · 2023-03-06T16:51:42Z

Label error. Requires exactly 0 of: Contingent - DNA, Needs Lead Approval, Contingent -- Do Not Approve. Found: enhancement, Contingent - DNA, 0 diff structural

…_GridComp into yv/feature/mom6-0.66deg

github-actions · 2023-03-06T17:21:33Z

Label error. Requires exactly 0 of: Contingent - DNA, Needs Lead Approval, Contingent -- Do Not Approve. Found: enhancement, Contingent - DNA, 0 diff structural

github-actions · 2023-03-06T19:37:50Z

Label error. Requires exactly 0 of: Contingent - DNA, Needs Lead Approval, Contingent -- Do Not Approve. Found: enhancement, Contingent - DNA, 0 diff structural

GEOSagcm_GridComp/GEOSphysics_GridComp/GEOSsurface_GridComp/Utils/Raster/makebcs/make_bcs

gmao-rreichle · 2023-03-06T19:48:43Z

@yvikhlya, @biljanaorescanin, @sdrabenh, @mathomp4:
I merged develop into Yury's branch through a command-line merge. This didn't even require explicitly resolving any conflicts. Git was apparently smart enough to recognize that we used "git mv" on "make_bcs" in our reorganization (#707) and then simply moved Yury's modified "make_bcs" into the new "./makebcs" subdir. Not sure why the merge on github indicated a conflict and complained about write permission for Biljana.
I added one more commit with some cleanup, see inline comment above.
This is now ok with me, and I'm happy for Matt to super-approve.
cc: @sanAkel

mathomp4

Re-approve

Rolf has said all is good.

sanAkel · 2023-03-07T02:04:02Z

FYI: @danholdaway, @Dooruk

sdrabenh and others added 4 commits December 16, 2022 14:56

Merge pull request #690 from GEOS-ESM/develop

7398a3d

Sync develop into main

Merge pull request #698 from GEOS-ESM/develop

377221d

Sync develop into main

make_bcs modified to add a new 0.66 degree MOM6 grid

2ac19b0

Merge branch 'develop' into yv/feature/mom6-0.66deg

077bb4d

yvikhlya added enhancement New feature or request 0 diff structural Structural changes to repository that are zero-diff labels Mar 2, 2023

yvikhlya requested a review from a team as a code owner March 2, 2023 20:13

yvikhlya self-assigned this Mar 2, 2023

yvikhlya requested a review from a team March 2, 2023 20:33

sanAkel added the Contingent - DNA These changes are contingent on other PRs (DNA=do not approve) label Mar 3, 2023

sanAkel self-requested a review March 3, 2023 14:44

sanAkel mentioned this pull request Mar 3, 2023

Add 0.66 MOM6 set up. GEOS-ESM/GEOS_OceanGridComp#21

Merged

sanAkel removed the Contingent - DNA These changes are contingent on other PRs (DNA=do not approve) label Mar 3, 2023

sanAkel reviewed Mar 6, 2023

View reviewed changes

GEOSagcm_GridComp/GEOSphysics_GridComp/GEOSsurface_GridComp/Utils/Raster/make_bcs Outdated Show resolved Hide resolved

Merge branch 'develop' into yv/feature/mom6-0.66deg

5b03ac9

sanAkel previously requested changes Mar 6, 2023

View reviewed changes

GEOSagcm_GridComp/GEOSphysics_GridComp/GEOSsurface_GridComp/Utils/Raster/make_bcs Outdated Show resolved Hide resolved

GEOSagcm_GridComp/GEOSphysics_GridComp/GEOSsurface_GridComp/Utils/Raster/make_bcs Outdated Show resolved Hide resolved

sanAkel reviewed Mar 6, 2023

View reviewed changes

gmao-rreichle added the Contingent - DNA These changes are contingent on other PRs (DNA=do not approve) label Mar 6, 2023

yvikhlya added 2 commits March 6, 2023 12:16

360x210 MOM6 removed from make_bcs. Fixed prompt for ocean model.

aee586c

Merge branch 'yv/feature/mom6-0.66deg' of github.com:GEOS-ESM/GEOSgcm…

69d1e8d

…_GridComp into yv/feature/mom6-0.66deg

gmao-rreichle added 2 commits March 6, 2023 14:20

Merge branch 'develop' into yv/feature/mom6-0.66deg

19d12f8

very minor MOM5/6 cleanup (MOM_VERSION init, comments, whitespace)

e4aa8dc

gmao-rreichle reviewed Mar 6, 2023

View reviewed changes

GEOSagcm_GridComp/GEOSphysics_GridComp/GEOSsurface_GridComp/Utils/Raster/makebcs/make_bcs Show resolved Hide resolved

gmao-rreichle removed the Contingent - DNA These changes are contingent on other PRs (DNA=do not approve) label Mar 6, 2023

mathomp4 self-requested a review March 6, 2023 20:17

mathomp4 approved these changes Mar 6, 2023

View reviewed changes

mathomp4 merged commit b513561 into develop Mar 6, 2023

mathomp4 deleted the yv/feature/mom6-0.66deg branch March 6, 2023 20:43

Dooruk mentioned this pull request Mar 7, 2023

A first crack at model (coupled or uncoupled) workflow GEOS-ESM/swell#148

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add new 0.66 degree MOM6 grid into make_bcs #712

Add new 0.66 degree MOM6 grid into make_bcs #712

yvikhlya commented Mar 2, 2023

gmao-rreichle commented Mar 2, 2023

sanAkel commented Mar 3, 2023

github-actions bot commented Mar 3, 2023

yvikhlya commented Mar 3, 2023

sanAkel commented Mar 3, 2023

biljanaorescanin commented Mar 6, 2023

sanAkel commented Mar 6, 2023 •

edited

Loading

sdrabenh commented Mar 6, 2023 •

edited

Loading

sanAkel Mar 6, 2023

sanAkel Mar 6, 2023

sdrabenh Mar 6, 2023

biljanaorescanin Mar 6, 2023

mathomp4 Mar 6, 2023

mathomp4 Mar 6, 2023

yvikhlya Mar 6, 2023

gmao-rreichle Mar 6, 2023

sanAkel Mar 6, 2023 •

edited

Loading

sanAkel Mar 6, 2023

gmao-rreichle commented Mar 6, 2023

sdrabenh commented Mar 6, 2023

yvikhlya commented Mar 6, 2023

yvikhlya commented Mar 6, 2023 •

edited

Loading

github-actions bot commented Mar 6, 2023

github-actions bot commented Mar 6, 2023

github-actions bot commented Mar 6, 2023

gmao-rreichle commented Mar 6, 2023

mathomp4 left a comment

sanAkel commented Mar 7, 2023

Add new 0.66 degree MOM6 grid into make_bcs #712

Add new 0.66 degree MOM6 grid into make_bcs #712

Conversation

yvikhlya commented Mar 2, 2023

gmao-rreichle commented Mar 2, 2023

sanAkel commented Mar 3, 2023

github-actions bot commented Mar 3, 2023

yvikhlya commented Mar 3, 2023

sanAkel commented Mar 3, 2023

biljanaorescanin commented Mar 6, 2023

sanAkel commented Mar 6, 2023 • edited Loading

sdrabenh commented Mar 6, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sanAkel Mar 6, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gmao-rreichle commented Mar 6, 2023

sdrabenh commented Mar 6, 2023

yvikhlya commented Mar 6, 2023

yvikhlya commented Mar 6, 2023 • edited Loading

github-actions bot commented Mar 6, 2023

github-actions bot commented Mar 6, 2023

github-actions bot commented Mar 6, 2023

gmao-rreichle commented Mar 6, 2023

mathomp4 left a comment

Choose a reason for hiding this comment

sanAkel commented Mar 7, 2023

sanAkel commented Mar 6, 2023 •

edited

Loading

sdrabenh commented Mar 6, 2023 •

edited

Loading

sanAkel Mar 6, 2023 •

edited

Loading

yvikhlya commented Mar 6, 2023 •

edited

Loading