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

Main namelist debug #671

Merged
merged 5 commits into from
Dec 8, 2021
Merged

Main namelist debug #671

merged 5 commits into from
Dec 8, 2021

Conversation

daveh150
Copy link
Contributor

@daveh150 daveh150 commented Dec 2, 2021

For detailed information about submitting Pull Requests (PRs) to the CICE-Consortium,
please refer to: https://github.com/CICE-Consortium/About-Us/wiki/Resource-Index#information-for-developers

PR checklist

  • Short (1 sentence) summary of your PR:
    Provide detailed output when namelist has wrong variables

  • Developer(s):
    David Hebert

  • Suggest PR reviewers from list in the column to the right.

  • Please copy the PR test results link or provide a summary of testing completed below.
    Quick suite BFB test on GAFFNEY shows:
    21 measured results of 21 total results
    21 of 21 tests PASSED
    0 of 21 tests PENDING
    0 of 21 tests MISSING data
    0 of 21 tests FAILED

  • How much do the PR code changes differ from the unmodified code?

    • [ x] bit for bit
    • different at roundoff level
    • more substantial
  • Does this PR create or have dependencies on Icepack or any other models?

    • [ x] Yes
    • No
  • Does this PR add any new test cases?

    • Yes
    • [x ] No
  • Is the documentation being updated? ("Documentation" includes information on the wiki or in the .rst files from doc/source/, which are used to create the online technical docs at https://readthedocs.org/projects/cice-consortium-cice/. A test build of the technical docs will be performed as part of the PR testing.)

    • Yes
    • [x ] No, does the documentation need to be updated at a later time?
      • Yes
      • [ x] No
  • [ x] Please provide any additional information or relevant details below:
    If you have a namelist variable incorrect (say CICE was updated with different namelist variable name), the error just tells you which namelist. This enhancement adds method to print the actual line in namelist.

Example: CICE6 changed 'sst_data_type' to 'ocn_data_type'. If you didn't change it new code will now print:

(abort_ice)ABORTED:
(abort_ice) called from ice_init.F90
(abort_ice) line number 579
(abort_ice) error =
(input_data)ERROR: reading namelist sst_data_type = 'gofs_daily'

@phil-blain
Copy link
Member

Hi David, I think this could be really useful. It's unfortunate that we have to repeat the code in several files, tough (but that would be a subject for another day!)

I noticed that the second commit is simply a fixup of the first one. You could use an interactive rebase to clean that up if you want.

I also noticed that you used a different email address for the two commits...

@apcraig
Copy link
Contributor

apcraig commented Dec 2, 2021

@daveh150. This is a nice feature to have. Knew we weren't handling this case well, but wasn't sure how to work around that. Nice update.

@eclare108213
Copy link
Contributor

I agree. Does it make sense to pull all these bits of identical code into subroutine in CICE_Finalize (or somewhere)?

@daveh150
Copy link
Contributor Author

daveh150 commented Dec 2, 2021 via email

@phil-blain
Copy link
Member

Yes, though you would have to push with --force since you would be changing history.

@apcraig
Copy link
Contributor

apcraig commented Dec 2, 2021

I don't think a common implementation belongs in CICE_Finalize, but maybe somewhere. In reality, this is just error trapping, is a few lines of code, and is implemented in a handful of places, I'm not sure we need to go for reuse.

One other thing. I might add a colon or something between the "ERROR" message and trim(tmpstr2) like

         call abort_ice(subname//'ERROR: reading icefields_nml: ' // &
              trim(tmpstr2), &
              file=__FILE__, line=__LINE__)

But that's a minor point overall and is fine as is. Otherwise, I think this looks good.

@daveh150
Copy link
Contributor Author

daveh150 commented Dec 2, 2021

I pushed the commit to cicecore/shared/ice_init_column.F90. I also did the interactive rebase Phillipe suggested. I did not need to use --force, but it seemed to work. :-)

if (nml_error /= 0) then
close (nu_nml)
call abort_ice(subname//'ERROR: reading icefields_drag_nml')
call abort_ice(subname//'ERROR: reading icefields_drag_nml' // &
Copy link
Contributor

Choose a reason for hiding this comment

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

Need a space at the end of the string like other calls.

if (nml_error /= 0) then
close (nu_nml)
call abort_ice(subname//'ERROR: reading icefields_fsd_nml')
call abort_ice(subname//'ERROR: reading icefields_fsd_nml'// &
Copy link
Contributor

Choose a reason for hiding this comment

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

need a space at the end of the ERROR string.

if (nml_error /= 0) then
close (nu_nml)
call abort_ice(subname//'ERROR: reading icefields_mechred_nml')
call abort_ice(subname//'ERROR: reading icefields_mechred_nml' // &
Copy link
Contributor

Choose a reason for hiding this comment

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

need a space at the end of the ERROR string.

if (nml_error /= 0) then
call abort_ice(subname//'ERROR: reading namelist', &
file=__FILE__, line=__LINE__)
call abort_ice(subname//'ERROR: reading namelist' // &
Copy link
Contributor

Choose a reason for hiding this comment

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

Need a space at the end of the ERROR string.

@apcraig
Copy link
Contributor

apcraig commented Dec 2, 2021

Sorry, an old comment got included in my review that wasn't supposed to be there. I have removed it.

@daveh150
Copy link
Contributor Author

daveh150 commented Dec 2, 2021

I added the needed spaces and colon for output messages.

@apcraig apcraig merged commit ddc1cfc into CICE-Consortium:main Dec 8, 2021
@phil-blain phil-blain mentioned this pull request Dec 21, 2021
16 tasks
apcraig added a commit to apcraig/CICE that referenced this pull request Mar 8, 2022
* update icepack, rename snwITDrdg to snwitdrdg (CICE-Consortium#658)

* Change max_blocks for rake tests on izumi (nothread). (CICE-Consortium#665)

* Fix some raketests for izumi

* fix some rake tests

* Makefile: make Fortran object files depend on their dependency files (CICE-Consortium#667)

When 'make' is invoked on the CICE Makefile, the first thing it does is
to try to make the included dependency files (*.d) (which are in fact
Makefiles themselves) [1], in alphabetical order.

The rule to make the dep files have the dependency generator, 'makdep',
as a prerequisite, so when processing the first dep file, make notices
'makdep' does not exist and proceeds to build it. If for whatever reason
this compilation fails, make will then proceed to the second dep file,
notice that it recently tried and failed to build its dependency
'makdep', give up on the second dep file, proceed to the third, and so
on.

In the end, no dep file is produced. Make then restarts itself and
proceeds to build the code, which of course fails catastrophically
because the Fortran source files are not compiled in the right order
because the dependency files are missing.

To avoid that, add a dependency on the dep file to the rules that make
the object file out of the Fortran source files. Since old-fashioned
suffix rules cannot have their own prerequisites [2], migrate the rules
for the Fortran source files to use pattern rules [3] instead. While at
it, also migrate the rule for the C source files.

With this new dependency, the builds abort early, before trying to
compile the Fortran sources, making it easier to understand what
has gone wrong.

Since we do not use suffix rules anymore, remove the '.SUFFIXES' line
that indicates which extension to use suffix rules for (but keep the
line that eliminates all default suffix rules).

[1] https://www.gnu.org/software/make/manual/html_node/Remaking-Makefiles.html
[2] https://www.gnu.org/software/make/manual/html_node/Suffix-Rules.html
[3] https://www.gnu.org/software/make/manual/html_node/Pattern-Rules.html#Pattern-Rules

* Fix multi-pe advection=none bug (CICE-Consortium#664)

* update parsing scripts to improve robustness, fix multi-pe advection=none

* Update cice script to improve performance including
minor refactoring of parse_namelist and parse_settings
to reduce cost and ability to use already setup ice_in file
from a prior case in the suite.

Added commented out timing ability in cice.setup.

Change test default to PEND from FAIL.

* fix cice.setup for case

* add sedbak implementation to support Mac sed

* s/spend/spent

* nuopc/cmeps driver updates (CICE-Consortium#668)


* add debug_model feature
* add required variables and calls for tr_snow

* Main namelist debug (CICE-Consortium#671)

* Adding method to write erroneous namelist options

* Remove erroneous comma in abort_ice for namelist check

* Added check for zbgc_nml. I missed that namelist in this file.

* Added space and colons for namelist error output

* Added space and colons for namelist error output

Co-authored-by: David A. Hebert <[email protected]>

* NUOPC/CMEPS cap updates (CICE-Consortium#670)

* updated orbital calculations needed for cesm

* fixed problems in updated orbital calculations needed for cesm

* update CICE6 to support coupling with UFS

* put in changes so that both ufsatm and cesm requirements for potential temperature and density are satisfied

* update icepack submodule

* Revert "update icepack submodule"

This reverts commit e70d1ab.

* update comp_ice.backend with temporary ice_timers fix

* Fix threading problem in init_bgc

* Fix additional OMP problems

* changes for coldstart running

* Move the forapps directory

* remove cesmcoupled ifdefs

* Fix logging issues for NUOPC

* removal of many cpp-ifdefs

* fix compile errors

* fixes to get cesm working

* fixed white space issue

* Add restart_coszen namelist option

* Update NUOPC cap to work with latest CICE6 master

* nuopc,cmeps or s2s build updates

* fixes for dbug_flag

* Update nuopc2 to latest CICE master

* Fix some merge problems

* Fix dbug variable

* Manual merge of UFS changes

* fixes to get CESM B1850 compset working

* refactored ice_prescribed_mod.F90 to work with cdeps rather than the mct data models

* Fix use_restart_time

* changes for creating masks at runtime

* added ice_mesh_mod

* implemented area correction factors as option

* more cleanup

* Fix dragio

* Fix mushy bug

* updates to nuopc cap to resolve inconsistency between driver inputs and cice namelists

* changed error message

* added icepack_warnings_flush

* updates to get ice categories working

* updates to have F compset almost working with cice6 - still problems in polar regions - need to resolve 253K/cice6 versus 273K/cice5 differences

* changed tolerance of mesh/grid comparison

* added issues raised in PR

* Update CESM-CICE sync with new time manager

* Add back in latlongrid

* Add new advanced snow physics to driver

* Fix restart issue with land blocks

* Update mesh check in cap

* fix scam problems

* reintroduced imesh_eps check

* Put dragio in the namelist instead

* Remove redundant code

* Fix some indents

Co-authored-by: Mariana Vertenstein <[email protected]>
Co-authored-by: apcraig <[email protected]>
Co-authored-by: Denise Worthen <[email protected]>

* Add CESM1_PIO for fill value check (CICE-Consortium#675)

* Add CESM1_PIO for fill value check

* Revert PIO_FILL_DOUBLE change for now

* - Update the namelist read to make the group order flexible. (CICE-Consortium#677)

- Remove recent update to namelist read that traps bad lines, it conflicts
  with flexibility to read groups in random order picked up by NAG.
- change print* statements to write(nu_diag,*)

* Port to Narwhal and add perf_suite (CICE-Consortium#678)

* Add narwhal intel, gnu, cray, aocc
Add perf_suite.ts

* update narwhal_cray and perf_suite

* Update OMP (CICE-Consortium#680)

* Add narwhal intel, gnu, cray, aocc
Add perf_suite.ts

* update narwhal_cray and perf_suite

* Review and update OMP implementation

- Fix call to timers in block loops
- Fix some OMP Private variables
- Test OMP Scheduling, add SCHEDULE(runtime) to some OMP loops
- Review column and advection OMP implementation
- ADD OMP_TIMERS CPP option (temporary) to time threaded sections
- Add timer_tmp timers (temporary)
- Add omp_suite.ts test suite
- Add ability to set OMP_SCHEDULE via options (ompscheds, ompscheds1, ompschedd1)

* - Review diagnostics OMP implementation
- Add timer_stats namelist to turn on extra timer output information
- Add ICE_BFBTYPE and update bit-for-bit comparison logic in scripts
- Update qc and logbfb testing
- Remove logbfb and qchkf tests, add cmplog, cmplogrest, cmprest set_env files to set ICE_BFBTYPE
- Update documentation

* Update EVP OMP implementation

* - Refactor puny/pi scalars in eap dynamics to improve performance
- Update OMP in evp and eap

* Clean up

* Comment out temporary timers

* Update OMP env variables on Narwhal

* Update gaffney OMP_STACKSIZE

* update OMP_STACKSIZE on cori

* Update Onyx OMP_STACKSIZE
Update documentation

* Update OMP_STACKSIZE on mustang

* - Update Tsfc values on land in various places in the code, was affecting testing.  Specifically fix upwind advection.
- Comment out OMP in ice_dyn_evp_1d_kernel, was producing non bit-for-bit results with different thread counts

* updating LICENSE.pdf for 2022

* seabed stress - remove if statements (CICE-Consortium#673)

* refactor seabed_stress. Bit for bit

* Removed if statement from stepu. Results are binary identical, however taubx and tauby is updated on all iterations instead of just the last one. Not used within iteration

* changed capping from logical to numeric in order to remove if statement. Moved call to deformation out of loop

* clean dyn_finish, correct intent(inout) to intent(in) for Cw, resse Cb in stepu, remove if from seabed_stress_LKD

* Reolve conflicts after updating main

* modified environment for Freya to accomodate for additional OMP commands

* Requested changes after review. Only changed in seabed stress and not bit for bit if cor=0.0

added legacy comment in ice_dyn_finish

* move deformation to subcycling

* - Update version and copyright. (CICE-Consortium#691)

- Remove gordon and conrad machines.
- Add setenv OMP_STACKSIZE commented out in env files
- Update Icepack to fc4b809

* add OMP_STACKSIZE for koehr (CICE-Consortium#693)

* Update C/CD deformations calls to be consistent with main B changes
Update tauxbx, tauxby calculations on C/CD to be consistent with main B changes

* Update OpenMP in C/CD implementation
Extend omp_suite to include C/CD tests

* reconcile recent merge problem

* set default value of capping to 0. in vp cases for backwards compatibility

* Set capping to 1.0 in vp consistent with evp, changes answers for vp configurations

Co-authored-by: David A. Bailey <[email protected]>
Co-authored-by: Philippe Blain <[email protected]>
Co-authored-by: Denise Worthen <[email protected]>
Co-authored-by: daveh150 <[email protected]>
Co-authored-by: David A. Hebert <[email protected]>
Co-authored-by: Mariana Vertenstein <[email protected]>
Co-authored-by: Elizabeth Hunke <[email protected]>
Co-authored-by: TRasmussen <[email protected]>
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.

4 participants