Fix history single/double precision issues #628
Conversation
- Fix io_netcdf. Double precision output was passed thru a single precision variable before writing so the precision in the output was lost. This is now fixed. Single and double precision netcdf output now reflects the internal model data correctly. - Fix io_pio2. Single precision output with pio2 was producing garbage for both netcdf and pnetcdf cases. This is not the case with pio1. Several changes were needed. - The iodesc initialization has to differentiate single or double target variables - The write_darray has to explicitly send a single or double array - Migrated to spval_dbl fills everywhere data is a double type, mostly in ice_history.F90. - Created a ice_write_hist_fill method in both io_netcdf and io_pio2 to improve reuse. Ran io_suite on cheyenne and checked that values are produced correctly for each case.
| call ice_pio_initdecomp(ndim3=nzilyr, ndim4=ncat_hist, iodesc=iodesc4di) | ||
| call ice_pio_initdecomp(ndim3=nzslyr, ndim4=ncat_hist, iodesc=iodesc4ds) | ||
| call ice_pio_initdecomp(ndim3=nfsd_hist, ndim4=ncat_hist, iodesc=iodesc4df) | ||
| call ice_pio_initdecomp(iodesc=iodesc2d, precision=history_precision) |
There was a problem hiding this comment.
I guess I didn't realize we didn't pass the precision argument here in CICE6. In the CICE5 driver, this is actually ice_precision= as an argument. That said, CICE6 still works with single precision history. See my output in:
/glade/scratch/dbailey/archive/b.e23.B1850.f09_g17.CMIP6-piControl.001_cice6/ice/hist
So, perhaps ice_precision is set internally in PIO somehow in the coupled system.
There was a problem hiding this comment.
I'm a little surprised that CICE5 and CICE6 were coded differently. I wonder how that happened. Were changes made in CICE5 in CESM for pio that never made it back to the official CICE repo? That information also makes it even more surprising that CICE6 is working properly in CESM with single precision history files thru pio2.
| status = pio_put_att(File, varid, 'missing_value', spval_dbl) | ||
| status = pio_put_att(File, varid,'_FillValue',spval_dbl) | ||
| endif | ||
| call ice_write_hist_fill(File,varid,coord_var(i)%short_name,history_precision) |
There was a problem hiding this comment.
I do like this idea. Makes the code little cleaner. There is still an outstanding issue where eliminated land blocks will not necessarily use the same fill value. Currently the land block default is zero in PIO.
There was a problem hiding this comment.
I can look into this in a future effort. I know there is also the idea of writing the grid data just once into it's own file, #613. I do worry that could break cf conventions (isn't each file supposed to be self documenting?). A discussion to be had another time probably.
| if (history_precision == 8) then | ||
| call pio_write_darray(File, varid, iodesc2d, & | ||
| workd2, status, fillval=spval_dbl) | ||
| else | ||
| workr2 = workd2 | ||
| call pio_write_darray(File, varid, iodesc2d, & | ||
| workr2, status, fillval=spval) | ||
| endif |
There was a problem hiding this comment.
might it make sense to also add a small subroutine (similarly to the new nice ice_write_hist_fill) for this bit of logic that is copy pasted a few times below ? This would increase code reuse...
Altough I see that it's used with arrays of different dimensions, so we might need a generic subroutine, maybe that would mean too much boilerplate.... what do you think ?
There was a problem hiding this comment.
Thanks @phil-blain, I thought about trying to introduce some reuse in the above code, but decided the payoff wasn't as straight-forward, as you note. If you think it's worthwhile, I'm happy to give it a shot.
There was a problem hiding this comment.
I'm not sure. I think you are right. If only Fortran had templated types like C++ :P
PR checklist
Short (1 sentence) summary of your PR:
Fix history single/double precision issues
Developer(s):
apcraig
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.
All tests pass bit-for-bit on cheyenne with 3 compilers, https://github.com/CICE-Consortium/Test-Results/wiki/cice_by_hash_forks#a779775c6e0cf1dd0f5153b8e97aeedda1b555de. The only failing test is the gx1prod test which is expected.
How much do the PR code changes differ from the unmodified code?
Does this PR create or have dependencies on Icepack or any other models?
Does this PR add any new test cases?
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.)
Please provide any additional information or relevant details below:
Fix io_netcdf. Double precision output was passed thru a single precision variable before writing so the precision in the output was lost. This is now fixed. Single and double precision netcdf output now reflects the internal model data correctly.
Fix io_pio2. Single precision output with pio2 was producing garbage for both netcdf and pnetcdf cases. This is not the case with pio1. Several changes were needed.
- The iodesc initialization has to differentiate single or double target variables
- The write_darray has to explicitly send a single or double array
Migrated to spval_dbl fills everywhere data is a double type, mostly in ice_history.F90.
Created a ice_write_hist_fill method in both io_netcdf and io_pio2 to improve reuse.
Ran io_suite on cheyenne and checked that values are produced correctly for each case.
Closes #625.