Update Derecho inteloneapi port and fix uninitialized variables - #561
Conversation
Update the inteloneapi compiler version on derecho to 2025.2.1. Update the intel and inteloneapi compiler debug options on derecho to better trap and warn. These changes are based on the UFS compiler settings and to help meet UFS requirements. The new settings are FFLAGS += -O0 -check all -check noarg_temp_created -warn -warn noerrors -fp-stack-check -fstack-protector-all -fpe0 -debug -ftrapuv -init=snan,arrays -link_mpi=dbg Fix several issues with uninitialized variables trapped with the new debug flags. For the most part, standalone Icepack results are bit-for-bit with the code changes except for Icepack bgc configurations because Sil and Nit were not initialized properly. Moved the init_forcing_bgc call earlier in the initialization calling sequence because sil_data and nit_data are needed in get_forcing_bgc which is called by init_bgc which is called by init_restart early-ish in the initialization sequencing. get_forcing_bgc is called again later in the initialization sequence and this seems redundant. It might be worth reviewing the bgc initialization. This was tested in standalone Icepack and standalone CICE on derecho with intel, inteloneapi, and gnu with the following results - Icepack derecho inteloneapi results are NOT bit-for-bit due to the updated compiler version on derecho with inteloneapi - Icepack derecho intel results are bit-for-bit for debug tests but are failing regression testing because fill values have changed on restart comparison. The log files are bit-for-bit. - Icepack bgc results are NOT bit-for-bit for all machines and compilers due to fixes in the bgc initialization in the Icepack driver. - These Icepack changes applied to CICE are all bit-for-bit in CICE, any bit-for-bit changes in standalone Icepack were implemented in the Icepack driver, not in columnphysics code.
with regression testing.
dabail10
left a comment
dabail10
left a comment
There was a problem hiding this comment.
Great catch on all of the uninitialized variables.
| istep = 0 ! local timestep number | ||
| time0=istep0*dt ! start time | ||
| time=istep0*dt ! s | ||
| time_forc=c0 ! forcing time, not used anymore |
There was a problem hiding this comment.
If this is not used anymore, can it be completed removed from the code?
There was a problem hiding this comment.
I wanted to remove it, but it is written to the restart file, so there might be a backwards compatibility issue with regard to those files. I think we could remove it with relative confidence that there wouldn't be a problem. I think the only case that could be a problem is using an older version of the code with a newer version of the restart file. There might also be an issue making sure the binary files are reading the right memory location. And finally, removing the variable from the restart file would mean regression testing (which compares restart files) would fail with the update, but that's something we can deal with. This implementation was the least risk, but I'd be happy to remove the variable completely.
There was a problem hiding this comment.
I see, it's probably best to just leave it alone for now. Maybe add to the comment "except in restart files for backward compatibility" or something like that.
There was a problem hiding this comment.
Good idea, I added some extra comments to the code.
3 participants
PR checklist
Short (1 sentence) summary of your PR:
Update Derecho inteloneapi port and fix uninitialized variables
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.
This was tested in standalone Icepack and standalone CICE on derecho with intel, inteloneapi, and gnu with the following results
How much do the PR code changes differ from the unmodified code?
Does this PR create or have dependencies on CICE 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/.)
Please document the changes in detail, including why the changes are made. This will become part of the PR commit log.
Update Derecho inteloneapi port.
Update the inteloneapi compiler version on derecho to 2025.2.1.
Update the intel and inteloneapi compiler debug options on derecho to better trap and warn. These changes are based on the UFS compiler settings and to help meet UFS requirements. The new settings are
Fix several issues with uninitialized variables trapped with the new debug flags. For the most part, standalone Icepack results are bit-for-bit with the code changes except for Icepack bgc configurations because Sil and Nit were not initialized properly. Moved the init_forcing_bgc call earlier in the initialization calling sequence because sil_data and nit_data are needed in get_forcing_bgc which is called by init_bgc which is called by init_restart early-ish in the initialization sequencing. get_forcing_bgc is called again later in the initialization sequence and this seems redundant. It might be worth reviewing the bgc initialization in standalone Icepack.
This was tested in standalone Icepack and standalone CICE on derecho with intel, inteloneapi, and gnu with the following results
- Icepack derecho inteloneapi results are NOT bit-for-bit due to the updated compiler version on derecho with inteloneapi
- Icepack bgc results are NOT bit-for-bit for all machines and compilers due to fixes in the bgc initialization in the Icepack driver.
- These Icepack changes applied to CICE are all bit-for-bit in CICE, any bit-for-bit changes in standalone Icepack were implemented in the Icepack driver, not in columnphysics code.