Add CESM1_PIO for fill value check #675
Conversation
|
Did you test the io options? The base suite doesn't test pio at all. You can either do it manually or run the io_suite which tests the netcdf, pio1, pio2, and binary options fairly comprehensively (only works on cheyenne because of pio requirement). Thanks. |
|
Sorry. I misunderstood. I thought you said the tests on cheyenne were failing and assumed the base_suite covered this. I will rerun with the io_suite. Dave |
|
Only the pio1 tests were failing and those are carried out weekly with the io_suite. Thanks! |
|
Any chance we could finish testing and merge before tomorrow (Fri, Dec 17) for weekend testing. I will also try to find a few minutes to fire off some tests as backup. |
|
I can definitely get this in. Am I using this flag incorrectly? Does CESM1_PIO mean PIO2 or PIO1? I would like this to be more clearly named. Dave |
|
I think the usage is correct. When CESM1_PIO is turned on, it's assuming it's compiled with pio from CESM1. |
|
So, why does this code not work? #ifndef CESM1_PIO |
|
In what way does it not work? Are you wondering if the where statement should work in PIO1 or are you asking why the ifdef still turns on the where statement with PIO1? |
|
The code does not compile because it cannot find PIO_FILL_DOUBLE. However, if CESM1_PIO is not defined, the code should be removed? I checked the pio1 cases and CESM1_PIO is not defined anywhere. |
|
Good point. I think I understand now. It looks like CESM1_PIO gets turned on by RASM (and should by other CESM1 versions when using this version of CICE and an older version of PIO). The pio1 we're testing on cheyenne is newer, so it supports the newer implementation of pio than what is needed for RASM. Sorry about my confusion. The CESM1_PIO is never turned on in CICE standalone testing on cheyenne, just in RASM testing. That's why it's not a PIO1 vs PIO2 thing. It's the slow evolution of PIO in ways that make it difficult to support backwards compatibility. So, it seems this new PIO_FILL_DOUBLE is also something that is newer than the pio1 we're testing on cheyenne which means it also will need to be turned off by CESM1_PIO. I guess, unfortunately, we need a new cpp for this in the standalone testing. So the options are to continue to introduce CPPs that support various versions
Or we could just try to implement this without using PIO_FILL_DOUBLE. For instance, could we have control of the fill value in PIO directly in CICE? That way CICE could set it to a special value, pass it to PIO, and then check for that special value at read-time. That might work in all versions of PIO too? |
|
Got it. The other issue that I do not have an answer to is Jim is calling the latest PIO2, and all previous versions are an iteration of PIO1. So, the code that we call PIO2 is actually PIO1. I wondered if we could just replace PIO_FILL_DOUBLE with spval_dbl. Let me think about it overnight and I'll look into a solution for tomorrow. |
|
Thanks @dabail10, let me know if I can help. |
|
So, the only issues with PIO1 are the setframe calls and PIO_FILL_DOUBLE. So, I would like to introduce a PIO1 flag and use this instead of CESM1_PIO. Would this work for RASM? |
|
I am also fine with CESM1_PIO and just making sure that we turn this on for our pio1 tests. |
|
So, this now works for PIO1, but it is breaking PIO2. The problem is when it tries to read iceumask (line 405): /glade/work/dbailey/CICE_ESCOMP/cicecore/cicedynB/infrastructure/ice_restart_driver.F90 It is aborting in ice_restart.F90 at the where statement, line 747. The only difference for iceumask is that it reads into work1 as a temporary. However, it is not happy with the check for PIO_FILL_DOUBLE. It works for the rest of the 2D fields though. Any thoughts? |
|
I don't see any reason iceumask would obviously fail. There doesn't seem to be anything special about it. Regarding use of the CPPs. The problem is we have 3 cases, so we need at least 2 CPPs. The three cases are
I wonder if at this point, it would make sense to remove the PIO_DOUBLE_FILL mods for now and check that in today? Then we can continue to explore how to deal with this best? I still sort of like the idea of having CICE set the fill/special value explicitly if possible and not use PIO_DOUBLE_FILL at all? That may not work in PIO. It could be that the fill value is not settable or it could be that the fill value for parts of the array that are not passed into PIO are not settable. We'd have to explore that. |
|
I'm fine with that. Gives me some more time to sort this out. |
|
I found this declaration in PIO2: double precision, public, parameter :: PIO_FILL_DOUBLE = 9.9692099683868690d+36 Weird that it is not the same as spval_dbl which is 1.0d30. I could do something like: where (work >= spval_dbl) work = c0 I was worried that checking if work was identically equal to PIO_FILL_DOUBLE that it might cause problems if roundoff different. Dave |
|
I found the problem with iceumask. It was because work1 was not initialized before it was sent to the read_restart_field subroutine. I assume the where statement choked because the ghost cells had nothing in them. I am rerunning the iosuite. |
|
I have run the quick suite and io suite on cheyenne with 3 compilers, everything is fine and bit-for-bit with the version from 2 weekends ago. I will merge now. |
* 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 <dhebert@nrlssc.navy.mil> * 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 <mvertens@ucar.edu> Co-authored-by: apcraig <anthony.p.craig@gmail.com> Co-authored-by: Denise Worthen <denise.worthen@noaa.gov> * 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 <dbailey@ucar.edu> Co-authored-by: Philippe Blain <philippe.blain@canada.ca> Co-authored-by: Denise Worthen <denise.worthen@noaa.gov> Co-authored-by: daveh150 <david.hebert@nrlssc.navy.mil> Co-authored-by: David A. Hebert <dhebert@nrlssc.navy.mil> Co-authored-by: Mariana Vertenstein <mvertens@ucar.edu> Co-authored-by: Elizabeth Hunke <eclare@lanl.gov> Co-authored-by: TRasmussen <33480590+TillRasmussen@users.noreply.github.com>
|
Fixed in #799 |
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
This accounts for the possibility of PIO1 which does not recognize PIO_FILL_DOUBLE.
dabail10 (D. Bailey)
https://github.com/CICE-Consortium/Test-Results/wiki/cice_by_hash_forks#7a1bd6982e9c8fec985ca3b2cbdd421dffe9492a (PGI had to be run separately)
I also ran aux_cice6 in CESM.
This fixes an issue with PIO1.