JCSDA changes #787
JCSDA changes #787
Conversation
danholdaway
requested review from
bensonr,
rem1776,
thomas-robinson and
wrongkindofdoctor
as code owners
|
@danholdaway - I noticed a good chunk of the standard FMS PR info is missing. Did the template not come up when you created the PR? The adjoint work was contributed by NASA. Are they aware of the adjoint comm changes? If not, we will need their approvals as well. |
|
@bensonr sorry about that, I think I accidentally deleted, I've included the rest of your template in the description. NASA is not aware about the adjoint changes since they were contributed by Jong Kim at EMC. I can work with the folks at GMAO to make sure their FV3 linear model works with these changes once they are part of develop. We run GMAO's linear model within the JCSDA so would just need to ship them the changes we've made there. |
mpp/include/mpp_do_update_ad.h
Outdated
| @@ -81,10 +80,11 @@ | |||
| send = recv | |||
|
|
|||
| if(debug_message_passing) then | |||
| nlist = size(domain%list(:)) | |||
| allocate(msg1(0:nlist-1), msg2(0:nlist-1) ) | |||
| nlist = size(domain%list(:)) | |||
There was a problem hiding this comment.
remove trailing whitespace
| @@ -159,7 +159,7 @@ module tracer_manager_mod | |||
| !> @{ | |||
|
|
|||
| integer :: num_tracer_fields = 0 | |||
| integer, parameter :: MAX_TRACER_FIELDS = 150 | |||
| integer, parameter :: MAX_TRACER_FIELDS = 250 | |||
There was a problem hiding this comment.
@GFDL-Eric can this be a namelist parameter?
There was a problem hiding this comment.
@thomas-robinson I don't believe we currently ingest a namelist for either tracer_manager or field_manager, although there are a set of tools in fm_utils.F90 that can "set" a namelist (fm_util_start_namelist), but it seems very ocean specific (hardcoded) and I'm not entirely sure what it's doing and/or if it's still even used. I don't know that we will have hardcoded limits like this after the rewrite, but that depends on how we implement things.
|
@danholdaway - have you confirmed the adjoint changes with NASA? |
|
@mathomp4 - Sorry to bother you. The developers at JCSDA have some updates to the adjoint code in FMS and I would appreciate you taking at look at this PR to ensure they changes has GMAO approval. Can you perform a review? |
@bensonr It's hard for me to review as we are a bit behind on FMS. We are in 2019.01 land, so my thought is: yes this is fine. I mean, any adjoint change is probably something I'd ask @danholdaway about anyway so, good to me? 😄 (Note: I am looking at trying to move GEOS to latest FMS but I got caught in a...spiral of CMake. I thought I'd try to take your CMakeLists.txt, but it's assuming a different build system than GEOS' so then I tried to convert Baselibs to build HDF4, HDF5, netCDF, etc with CMake and I'm hitting bugs in their build systems. I currently am needing to use the development branches of HDF5 and netCDF-C to get close to building with CMake... We might have to make a "plug" around FMS and maybe override the CMake like we do with MOM6, etc.) |
|
Update: I'm going to try just pulling in these changes from @danholdaway into our FMS as a test to see if it builds and works with the GCM. My guess is it will since that won't trigger the adjoint. I might have to work with people on our end to figure out how to actually trigger that code in an ADAS run. |
These are updates based on: NOAA-GFDL#787
|
@mathomp4 - thanks for the messages. I'll await hearing from you on your attempts to bring in the adjoint updates and whether they are acceptable for NASA. The CMake system was contributed by @aerorahul and was purpose built for the NEMS/UFS build systems. It looks like NASA has gone forward with a CMake system and the autotools config/build system is not of interest. Is that correct? |
|
@bensonr GEOS uses @mathomp4 |
|
Correct me if I'm wrong @mathomp4 but this PR does not need to wait on GMAO testing their adjoint with these changes. GMAO has two modes of building: GEOSgcm and GEOSadas. The GEOSadas build (which contains the adjoint) will likely never need to be updated to use a version of FMS that includes these changes. The adjoint in that system will be superseded by the adjoint coming from JEDI in the not too distant future and that will already be tested with these changes. |
|
@aerorahul Yeah. I figured out a way to do things with the Autotools build of FMS. The CMake build was a rabbit hole (I think I found a bug in netCDF's CMake build), so I switched to the old standby of autotools. When I did that, I got farther. I even got our old FV3 to build but got stuck (surprisingly) on MOM6 which is essentially pretty close to I'll be talking with @sanAkel on that issue. I'm sure it's just a "Matt doesn't know MOM6" sort of thing that can be fixed up (files need added our CMakeLists, etc.) |
|
@mathomp4 |
@mathomp4 And all (interested), it has only recently (about a week ago) that I heard GMAO/GEOS folks are finally getting caught up with FMS. Due to that reason, we (at GMAO) use MOM6 interface to |
|
@aerorahul <https://github.com/aerorahul> Yeah. I figured out a way to do
things with the Autotools build of FMS. The CMake build was a rabbit hole
(I think I found a bug in netCDF's CMake build), so I switched to the old
standby of autotools.
When I did that, I got farther. I even got our old FV3 to build but got
stuck (surprisingly) on MOM6 which is essentially pretty close to dev/gfdl
.
I'll be talking with @sanAkel <https://github.com/sanAkel> on that issue.
I'm sure it's just a "Matt doesn't know MOM6" sort of thing that can be
fixed up (files need added our CMakeLists, etc.)
@mathomp4 <https://github.com/mathomp4> And all (interested), it has only
recently (about a week ago) that I heard GMAO/GEOS folks are *finally*
getting *caught* up with FMS. Due to that reason, we (at GMAO) use MOM6
interface to FMS1. I can work to switch to FMS2 which follows all the
MOM6 infra changes already in place
<https://github.com/NOAA-GFDL/MOM6/pull/1359>.
@sanAkel - while FMS does have the newer fms2_io layer, the older
fms_io/mpp_io interfaces, although deprecated, still exist and can be used
simultaneously with fms2_io.
|
Yes. I was able to build FMS with CMake (with a hack shown in #806 for PIC) and then I had to do some rather ugly hacking in our Now of course, in truth I'll have to update our fork because of our constants (see #155, #796) and re-do all this with that (I'm using this repo for now), but we are closer! |
|
Yes. I was able to build FMS with CMake (with a hack shown in #806
<#806> for PIC) and then I had to
do some rather ugly hacking in our FindBaselibs code where I have to
"make my own FMS::fms_r4 targets" and the like. But after that things
seem to build. I hav
Now of course, in truth I'll have to update our fork because of our
constants (see #155 <#155>, #796
<#796>) and re-do all this with
that (I'm using this repo for now), but we are closer!
@mathomp4 - now is a good time to further the discussion on a proper path
for constants. We can reopen the discussion in #796
<#796>. I have two ideas and would
welcome input from you and @aerorahul on them.
|
|
Just to try and see what happens, I tried a stock coupled model run.
Any thoughts?? |
|
Hi Santha,
It's not super clear to me why we're reserving unit 9 for the etc_unit, but
I would think we'd be susceptible to running into that error depending on
how many files are opened before the mpp_init call. Thoughts on why we've
got this unit reserved, Rusty?
For the first error, grid2 checks the grid_spec.nc file for the existence
of the variables atm_mosaic_file, ocn_mosaic_file, lnd_mosaic_file... if
those variables exist within grid_spec.nc, grid2 will attempt to open those
files and will throw an error if it is unable to do so. So either
grid_spec.nc would need to be changed to not reference those files... or
you could do exactly as you did and make those files.
Eric
…On Thu, Sep 2, 2021 at 9:55 AM Santha Akella ***@***.***> wrote:
@bensonr <https://github.com/bensonr>
Just to try and see what happens, I tried a *stock* coupled model run.
- First it died looking for INPUT/atmos_mosaic.nc and INPUT/
land_mosaic.nc We did not use them before in GEOS coupled (or even
uncoupled: AGCM-only) models. In any case, since I have ocean-only set up
from MOM6-examples and my own config, I gave it some xx_mosaic.nc;
xx=atmos, land and it moved past that failure. In debug mode,
Image PC Routine Line Source
GEOSgcm.x 00000000102E0286 Unknown Unknown Unknown
GEOSgcm.x 000000000EC48805 mpp_mod_mp_mpp_er 71 mpp_util_mpi.inc
GEOSgcm.x 000000000EB3FFB2 fms_io_utils_mod_ 201 fms_io_utils.F90
GEOSgcm.x 000000000EB8E166 netcdf_io_mod_mp_ 367 netcdf_io.F90
GEOSgcm.x 000000000EBAB4B8 netcdf_io_mod_mp_ 643 netcdf_io.F90
GEOSgcm.x 000000000EBAA9BD netcdf_io_mod_mp_ 1961 netcdf_io.F90
GEOSgcm.x 000000000EBE59E5 grid2_mod_mp_open 180 grid2.F90
GEOSgcm.x 000000000EBF2852 grid2_mod_mp_open 195 grid2.F90
GEOSgcm.x 000000000EBF3AD6 grid2_mod_mp_open 252 grid2.F90
GEOSgcm.x 000000000EBF3C8D grid2_mod_mp_grid 144 grid2.F90
GEOSgcm.x 000000000EB2EED4 fms_mod_mp_fms_in 435 fms.F90
- But then died with following: Unit 9 is already in use (etc_unit) in
mpp_comm_mpi here is the trace:
Image PC Routine Line Source
GEOSgcm.x 00000000102E0286 Unknown Unknown Unknown
libMOM6_GEOSPlug. 00002AAE5CCA5025 mpp_mod_mp_mpp_er 71 mpp_util_mpi.inc
libMOM6_GEOSPlug. 00002AAE5CCA1FD5 mpp_mod_mp_mpp_in 140 mpp_comm_mpi.inc
libMOM6_GEOSPlug. 00002AAE5CB9C46C fms_mod_mp_fms_in 343 fms.F90
libMOM6_GEOSPlug. 00002AAE5CA8D214 mom6_geosplugmod_ 709 MOM6_GEOSPlug.F90
libesmf.so 00002AAABA568344 _ZN5ESMCI6FTable1 Unknown Unknown
libesmf.so 00002AAABA56BE8F ESMCI_FTableCallE Unknown Unknown
Any thoughts??
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#787 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AB3MSXMYQOOPRHVTLXTXFBDT7565HANCNFSM5A4DDHLQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
|
@GFDL-Eric 👋 Reg.
As you say, let's defer to @bensonr. As for:
I get it, thank you! Alright, with MOM6, since the |
|
Hi Santha,
It's not super clear to me why we're reserving unit 9 for the etc_unit, but
I would think we'd be susceptible to running into that error depending on
how many files are opened before the mpp_init call. Thoughts on why we've
got this unit reserved, Rusty?
For the first error, grid2 checks the grid_spec.nc file for the existence
of the variables atm_mosaic_file, ocn_mosaic_file, lnd_mosaic_file... if
those variables exist within grid_spec.nc, grid2 will attempt to open
those
files and will throw an error if it is unable to do so. So either
grid_spec.nc would need to be changed to not reference those files... or
you could do exactly as you did and make those files.
Eric
The original mpp_io/fms_io had a lower limit for range of unit id's it
would manage. Using 9 for etc_unit was below the range and seen as a
"safe" choice. With fms2_io using the "newunit" keyword in open's, we can
either update the code that relied on etc_unit to utilize the newunit
keyword or force the etc_unit to be opened during mpp_init to ensure the
unit is free. My druthers would be to utilize the newunit keyword. We
should also ensure the other older unit-handling functions are scrubbed as
well. Eric - your thoughts?
… |
|
Rusty,
I would vastly prefer to move everything to newunit.
Eric
…On Thu, Sep 2, 2021 at 10:53 AM Rusty Benson ***@***.***> wrote:
>
> Hi Santha,
>
> It's not super clear to me why we're reserving unit 9 for the etc_unit,
but
> I would think we'd be susceptible to running into that error depending on
> how many files are opened before the mpp_init call. Thoughts on why we've
> got this unit reserved, Rusty?
>
> For the first error, grid2 checks the grid_spec.nc file for the
existence
> of the variables atm_mosaic_file, ocn_mosaic_file, lnd_mosaic_file... if
> those variables exist within grid_spec.nc, grid2 will attempt to open
> those
> files and will throw an error if it is unable to do so. So either
> grid_spec.nc would need to be changed to not reference those files... or
> you could do exactly as you did and make those files.
>
> Eric
>
The original mpp_io/fms_io had a lower limit for range of unit id's it
would manage. Using 9 for etc_unit was below the range and seen as a
"safe" choice. With fms2_io using the "newunit" keyword in open's, we can
either update the code that relied on etc_unit to utilize the newunit
keyword or force the etc_unit to be opened during mpp_init to ensure the
unit is free. My druthers would be to utilize the newunit keyword. We
should also ensure the other older unit-handling functions are scrubbed as
well. Eric - your thoughts?
>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#787 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AB3MSXJYVYR4QQEVUK5XK4TT76FWZANCNFSM5A4DDHLQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
|
I would vastly prefer to move everything to newunit.
Eric
@GFDL-Eric - ss this is somewhat orthogonal to the PR, please open a new
issue and assign it for resolution as appropriate.
… |
|
Note: I got @sanAkel past the &mpp_nml
etc_unit_is_stderr = .TRUE.
/in |
|
Superseded by #807 so closing. |
Description
This a series of changes implemented by the JCSDA to enable data assimilation applications.
The main changes are to allow for paths to the namelist files, field_table, and the INPUT directory. The solution used follows the one suggested provided to @aerorahul by @bensonr.
Other change include allowing more tracers for EMC's CMAQ assimilation and changes to MPP adjoint for data assimilation with MOM6
How Has This Been Tested?
We have run the the unit tests of the JEDI system
Checklist:
make distcheckpasses