Add nuopc cap #44
Add nuopc cap #44
Conversation
…-wrapper into mvertens/nuopc_cap
|
@jedwards4b - If we resolved the issue of CISM components writing to stdout, would that make it unnecessary to change the default setting? I do, in fact, often use this setting. I just submitted a paper, which means I now have some time to work on deferred CISM maintenance, in coordination with @billsacks. |
Mariana isn't sure why she removed these conditionals.
This module was duplicated between mct and nuopc.
This points to a commit that fixes the run sequence for I compsets.
|
@billsacks - I wanted to follow up on the question I asked last week: If we resolved the issue of CISM components writing to stdout, would that make it unnecessary to change the default setting? I'm happy to update the write statements in Glissade. I'd just like to be clear on the required changes, and was wondering if I should fold these CISM changes into my work on multiple instances. |
|
@whlipscomb I'm not entirely sure what @jedwards4b wanted, but based on ESCOMP/CISM#32, I'm thinking: (1) As @jedwards4b said, print * should be replaced with write(stdout, *). I think that, in a CESM run, this leads to messages appearing in different files depending on whether they are written by the master task or a different task. (2) If you notice any writes that are done on all tasks but really just need to be done on master – or not at all – then changing the code appropriately. If these changes are extensive, then it could be better to put them in a different PR, though if you prefer to put them in the same PR that's okay, but I'd suggest at least doing them in separate commits to aid reviewability of the PR. |
|
Thank you for addressing this issue. Note that item (2) of Bill's message can be accomplished by simply changing the default log level. We should really try to avoid output from all processors whenever possible. |
|
Thanks, @billsacks and @jedwards4b, I can work on this in the next couple of weeks. Besides fixing print statements, I'll try to avoid writing output from more than one processor. |
This will be less prone to subtle errors in case of typos. See ESCOMP/CTSM#1083 for detailed thoughts.
| @@ -189,7 +190,7 @@ subroutine glc_initialize(EClock) | |||
| paramfile = 'unknown_paramfile' | |||
|
|
|||
| if (my_task == master_task) then | |||
| open (nml_in, file=nml_filename, status='old',iostat=nml_error) | |||
| open (newunit=nml_in, file=nml_filename, status='old',iostat=nml_error) | |||
There was a problem hiding this comment.
This is a Fortran 2008 feature, but @mvertens feels it is well supported by the major compilers at this point, and it has been exercised heavily in cmeps testing.
I am removing this todo: TODO (mvertens, 2018-11-28): Determine if land is present as a sanity check - do we need this? If I remember correctly, I only added that sanity check because E3SM introduced the ability for GLC to be run with SLND with MCT (CPL7), and I wanted to make sure CISM wasn't accidentally used in that configuration.
drivers/cpl/nuopc/glc_comp_nuopc.F90
Outdated
| ! write(6,102) abs(mesh_lons(n) - lons_vec(n)) | ||
| !102 format('ERROR: CISM lon diff = ',f20.10,' is too large') | ||
| !call shr_sys_abort() |
There was a problem hiding this comment.
- @mvertens - is there a reason that the aborts are commented out for the various checks here – on lons, lats & areas? Maybe we don't want to compare areas, but should we at least be comparing lats and lons? It seems like either we should uncomment these commented lines or stop comparing lats & lons at all.
drivers/cpl/nuopc/glc_comp_nuopc.F90
Outdated
| ! elemAreaArray = ESMF_ArrayCreate(DistGrid, mesh_areas, rc=rc) | ||
| ! if (ChkErr(rc,__LINE__,u_FILE_u)) return | ||
| ! call ESMF_MeshGet(Emesh, elemAreaArray=elemAreaArray, rc=rc) | ||
| ! if (ChkErr(rc,__LINE__,u_FILE_u)) return |
There was a problem hiding this comment.
- @mvertens - should we remove this commented-out code, or will you be making use of it soon?
|
It should be removed - and it is in my updated branch.
…On Tue, Nov 24, 2020 at 1:09 PM Bill Sacks ***@***.***> wrote:
***@***.**** commented on this pull request.
@mvertens <https://github.com/mvertens> please see a couple of questions
below about whether we should be doing some consistency checks
------------------------------
In drivers/cpl/nuopc/glc_comp_nuopc.F90
<#44 (comment)>:
> + ! write(6,102) abs(mesh_lons(n) - lons_vec(n))
+ !102 format('ERROR: CISM lon diff = ',f20.10,' is too large')
+ !call shr_sys_abort()
- @mvertens <https://github.com/mvertens> - is there a reason that the
aborts are commented out for the various checks here – on lons, lats &
areas? Maybe we don't want to compare areas, but should we at least be
comparing lats and lons? It seems like either we should uncomment these
commented lines or stop comparing lats & lons at all.
------------------------------
In drivers/cpl/nuopc/glc_comp_nuopc.F90
<#44 (comment)>:
> + ! elemAreaArray = ESMF_ArrayCreate(DistGrid, mesh_areas, rc=rc)
+ ! if (ChkErr(rc,__LINE__,u_FILE_u)) return
+ ! call ESMF_MeshGet(Emesh, elemAreaArray=elemAreaArray, rc=rc)
+ ! if (ChkErr(rc,__LINE__,u_FILE_u)) return
- @mvertens <https://github.com/mvertens> - should we remove this
commented-out code, or will you be making use of it soon?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#44 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AB4XCE3NFMZNG6N4ZLBAU33SRQHHXANCNFSM4TSQ2MZQ>
.
--
Mariana Vertenstein
CESM Software Engineering Group Head
National Center for Atmospheric Research
Boulder, Colorado
Office 303-497-1349
Email: mvertens@ucar.edu
|
Mariana Vertenstein says these error checks for consistency between internal and mesh lats & lons should probably be active.
These fix build issues
Resolved Conflicts: drivers/cpl/nuopc/glc_comp_nuopc.F90 drivers/cpl/nuopc/glc_import_export.F90
This includes Mariana's fixes to SMB renormalization
Resolved Conflicts: Externals.cfg
List the various lines in a consistent order, and use the long hash
This duplicates all tests EXCEPT: - CISM1 (G1 in compset alias) - izumi
According to Jim Edwards, NCK tests are not expected to work with nuopc, and indeed I found that there were roundoff-level differences between the single and multi-instance versions of that test (maybe due to a difference in number of processors?). Jim Edwards suggested changing this to MCC, so I have done this here; this MCC test passes.
This test fails, but I don't think it's very important: I don't think this is a configuration that will actually be run for scientific purposes. I have used it for testing purposes to test the noevolve logic, but I don't think it's very important for that: we can test this via an I compset. (And in any case, noevolve will be a less important configuration once we have a data glc model to serve that purpose.) See ESCOMP/CMEPS#142 for more comments.
These tests die with the error: * FATAL ERROR : (glad_main.F90:650) Valid_inputs cannot be .false. if trying to do a mass balance time step I think this is a known limitation of CMEPS right now, but I will open an issue to track this - and then to reinstate these tests when the issue is fixed.
Opening this on behalf of @mvertens . I'll review it and then bring it to master.