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

Closes #70. Commit changes for PBL height #71

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

gmao-yzhu
Copy link

I pushed changes for the PBL height assimilation. Please help to review the code changes, and let me know if I need to make any modifications. Thanks in advance.

@gmao-yzhu gmao-yzhu requested a review from a team as a code owner March 10, 2021 19:06
@mathomp4 mathomp4 added Needs Lead Approval Change requires science lead approval and removed Needs Lead Approval Change requires science lead approval labels Mar 11, 2021
Copy link
Collaborator

@rtodling rtodling left a comment

Choose a reason for hiding this comment

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

Yanqiu,
It seems to me you placed the pblh field in the less desirable place. it seems to me you added the field to the bkg.eta files instead of the bkg.sfc file. For one thing, PBLH is a 2d field that fits better w/in the context of bkg.sfc ... but more importantly is the fact that changing the bkg.eta has consequences that are too complex and long to explain here.

I suggest you revise all your changes and move PBLH from bkg.eta to bkg.sfc ...

in the context of this particular file (GSI_GridComp.rc.tmpl it means moving it from vars_bkgeta_file and vars_anaeta_file to vars_bkgsfc_file ... and we'll need to think (and add) a vars_anasfc_file; presently GSI does not produce such an output but it should be possible to come up w/ that. I can help you do this part since it will be not so straightforward.

@rtodling
Copy link
Collaborator

Yanqiu,
I am also confused as to why this PR only has two files changed! Also, it seem to me you don't know this, but the file GSI Gaussian_GridComp you changed is not used in GEOS; so I would not worry about this file - it provides (a now outdated) hook to the NCEP bkg files ...

@gmao-yzhu
Copy link
Author

gmao-yzhu commented Apr 14, 2021 via email

@gmao-yzhu
Copy link
Author

I will check why only two files changes were shown. Thanks Ricardo.

@gmao-yzhu
Copy link
Author

hum, I am not sure what happened. It is shown the following when I did "mepo status":
Checking status...
GEOSadas | (b) feature/yzhu/PBL_height
env | (t) v1.4.2 (DH)
cmake | (t) v1.0.11 (DH)
ecbuild | (t) geos/v1.0.0 (DH)
NCEP_Shared | (t) v1.1.0 (DH)
GMAO_Shared | (b) feature/yzhu/PBL_height
MAPL | (t) v1.1.14 (DH)
FMS | (t) geos/orphan/v1.0.3 (DH)
GEOSana_GridComp | (b) feature/yzhu/PBL_height
GEOSgcm_GridComp | (t) cvs/GEOSadas-5_27_1_p3 (DH)
g5pert | (t) v1.1.2 (DH)
GEOSagcmPert_GridComp | (t) v1.1.0 (DH)
FVdycoreCubed_GridComp | (t) v1.0.9 (DH)
fvdycore | (t) geos/v1.0.2 (DH)
GEOSchem_GridComp | (t) v1.1.1 (DH)
mom | (t) geos/v1.0.1 (DH)
GEOSgcm_App | (t) cvs/GEOSadas-5_27_1_p3 (DH)
UMD_Etc | (t) v1.0.2 (DH)
CPLFCST_Etc | (t) v1.0.1 (DH)

After I typed "mepo push GEOSadas GMAO_Shared GEOSana_GridComp", it is shown the following

Pushed: GEOSadas

Branch 'feature/yzhu/PBL_height' set up to track remote branch 'feature/yzhu/PBL_height' from '[email protected]:GEOS-ESM/GEOSadas.git'.
Everything up-to-date

Pushed: GMAO_Shared

Branch 'feature/yzhu/PBL_height' set up to track remote branch 'feature/yzhu/PBL_height' from '[email protected]:GEOS-ESM/GMAO_Shared.git'.
Everything up-to-date

Pushed: GEOSana_GridComp

Branch 'feature/yzhu/PBL_height' set up to track remote branch 'feature/yzhu/PBL_height' from '[email protected]:GEOS-ESM/GEOSana_GridComp.git'.
Everything up-to-date

I will talk to Matt to find out what happened.

@gmao-yzhu
Copy link
Author

Ricardo, Please see below Matt's email. He confirmed that all my commits. have been pushed to GitHub. I need to make pull request for GEOSadas, GEOSana_Gridcomp and GMAO_Shared respectively.

PS Matt's email:
Yanqiu,

As far as I can see, all your commits have been pushed to GitHub.

There is this PR:

#71

and then on GMAO_Shared I see this:

GEOS-ESM/GMAO_Shared@release/cvs/GEOSadas-5_27_1_p2...feature/yzhu/PBL_height

and GEOSana_Gridcomp there is:

GEOS-ESM/GEOSana_GridComp@develop...feature/yzhu/PBL_height

It's up to you to make the Pull Requests on all repositories where you want code to be merged. Mepo will only push, but it can't really do anything "fancy" on GitHub, just push/pull code.

Matt

@rtodling
Copy link
Collaborator

rtodling commented Apr 15, 2021

Yanqiu,

Unfortunately, PBLH will need to move the bkg.sfc. Only I know the complexity and the can of worms that adding to bkg.eta will be.

I will look at the full PR - on all repos in consideration - when the above has been addressed. The change above will sip through the various repos related to this PR.

Ricardo

@gmao-yzhu
Copy link
Author

OK, thanks Ricardo, it is fine with me to move PBLH from bkg.eta to bkg.sfc in order to avoid any possible issues/complications. Please help with the part you mentioned above.

Thanks in advance.
Yanqiu

@rtodling
Copy link
Collaborator

Yanqiu, I have looked a little more closely about the suggestion to have PBLH moved to bkg.sfc ... indeed, quite sometime back I had already added PBLH to the bkg.sfc files (if you look at the output of x0042/43/44 all have PBLH in the file. But as I look more closely I am finding there will be some considerable challenges having the field in the the bkg.sfc files; one of which related to having to read the sfc fields in the ensemble part in GSI.

So let me look again at what you have - with the field placed in the bkg.eta files ... I will look into what needs to be done to preserve some level of backward compatibility,

I imagine (hope) you haven't done too much yet in trying to handle things from the bkg.sfc side. Sorry for flipping my mind.

@gmao-yzhu
Copy link
Author

Ricardo, Thank you very much for looking into this closely. I haven't done anything with bkg.sfc, so no worries. You know GEOS better than me, so I will rely on your judgement for the bkg.eta/bkg.sfc decision.

Yanqiu

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