Update Derecho Cray, rename subroutines, minor cleanup #502
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Update Derecho Cray, add -Rp to -O2 standard optimization to address
SAME_TBS compiler bug that is triggered in limited cases. Reported
issue to NCAR. -O2 -Rp changes answers relative to -O2.
- Rename two public Icepack subroutines, but maintain backward compatibility,
use icepack_therm_shared , only: icepack_init_thermo => icepack_init_salinity
use icepack_therm_shared , only: icepack_init_trcr => icepack_init_enthalpy
Update the calls in Icepack driver, but also confirmed it works fine with the old
names as well as in CICE with the old names.
Closes CICE-Consortium#255
- Add a check and abort for negative values in the sqrt in computation of Tin in
function calculate_Tin_from_qin.
Closes CICE-Consortium#482
- Refactor calls to icepack_aggregate to make them consistent. This was part of the
testing for the Derecho Cray bug, and decided to keep the implementation.
- Update comments associated with floeshape constant attribution. Change from
Steele to Rothrock 1984.
Closes CICE-Consortium#479
- Clean up some of the variable declarations in subroutine set_state_var and module
icedrv_state, merge multiple lines to one line and shift to assumed shape arrays
where appropriate.
- Clean up implementation error in icedrv_restart.F90, subroutine restartfile. This subroutine
was using a parameter, ntrcr, directly from icepack_tracers. Switched that to a call to
icepack_query_tracer_sizes.
eclare108213
reviewed
Oct 3, 2024
There was a problem hiding this comment.
Just one very minor comment on the current changes. If it makes sense to you @apcraig, this PR could capture a few other minor cleanup kind of things e.g.
- comments/documentation for kice need to say that it is specific to the MU71 conductivity (mentioned in kice in mushy thermodynamics #447, not enough to close that issue)
- a one-character correction: Possible problem in ice_restoring if hbar=hin_min or hin_max then restoring to zero CICE#972
Or those can wait to go into a different PR.
|
I updated the icepack_init_salinity documentation and the kice documentation and note those in the PR now. CICE-Consortium/CICE#972 is a CICE issue and will need to be fixed in a CICE PR. I also updated the interface documentation (overdue). None of the most recent changes affect any code or scripts (just comments and documentation) so no retesting has been done. |
|
@eclare108213 I updated and think this is ready to merge, can you have a look. |
eclare108213
approved these changes
Oct 11, 2024
dabail10
pushed a commit
to ESCOMP/Icepack
that referenced
this pull request
Oct 25, 2024
…um#502) * Update Derecho Cray, rename subroutines, minor cleanup Update Derecho Cray, add -Rp to -O2 standard optimization to address SAME_TBS compiler bug that is triggered in limited cases. Reported issue to NCAR. -O2 -Rp changes answers relative to -O2. Rename two public Icepack subroutines, but maintain backward compatibility. Update the calls in Icepack driver, but also confirmed it works fine with the old names as well as in CICE with the old names. Closes CICE-Consortium#255 use icepack_therm_shared , only: icepack_init_thermo => icepack_init_salinity use icepack_therm_shared , only: icepack_init_trcr => icepack_init_enthalpy Add a check and abort for negative values in the sqrt in computation of Tin in function calculate_Tin_from_qin. Closes CICE-Consortium#482 Refactor calls to icepack_aggregate to make them consistent. This was part of the testing for the Derecho Cray bug, and decided to keep the implementation. Update comments associated with floeshape constant attribution. Change from Steele to Rothrock 1984. Closes CICE-Consortium#479 Clean up some of the variable declarations in subroutine set_state_var and module icedrv_state, merge multiple lines to one line and shift to assumed shape arrays where appropriate. Clean up implementation error in icedrv_restart.F90, subroutine restartfile. This subroutine was using a parameter, ntrcr, directly from icepack_tracers. Switched that to a call to icepack_query_tracer_sizes. Update documentation of kice noting it's use with BL99 and MU71. See CICE-Consortium#447. Generate updated interface documentation (./icepack.setup --docintfc)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR checklist
Short (1 sentence) summary of your PR:
Update Derecho Cray, rename subroutines, minor cleanup
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.
Full test suite on Derecho with Icepack passes all tests. Derecho cray results changes answers due to compiler changes. https://github.com/CICE-Consortium/Test-Results/wiki/icepack_by_hash_forks#ba0bc0b8f76d965aa4186bc3e4ffc7255a91e7fb. Also tested these changes in CICE, everything is bit-for-bit.
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 Cray, add -Rp to -O2 standard optimization to address SAME_TBS compiler bug that is triggered in limited cases. Reported issue to NCAR. -O2 -Rp changes answers relative to -O2.
Rename two public Icepack subroutines, but maintain backward compatibility. Update the calls in Icepack driver, but also confirmed it works fine with the old names as well as in CICE with the old names.
Closes Icepack Interface Refactor #255
Add a check and abort for negative values in the sqrt in computation of Tin in function calculate_Tin_from_qin.
Closes Undefined behaviour in
calculate_Tin_from_qin#482Refactor calls to icepack_aggregate to make them consistent. This was part of the testing for the Derecho Cray bug, and decided to keep the implementation.
Update comments associated with floeshape constant attribution. Change from Steele to Rothrock 1984.
Closes Incorrect floe size reference #479
Clean up some of the variable declarations in subroutine set_state_var and module icedrv_state, merge multiple lines to one line and shift to assumed shape arrays where appropriate.
Clean up implementation error in icedrv_restart.F90, subroutine restartfile. This subroutine was using a parameter, ntrcr, directly from icepack_tracers. Switched that to a call to icepack_query_tracer_sizes.
Update documentation of kice noting it's use with BL99 and MU71. See kice in mushy thermodynamics #447.
Generate updated interface documentation (./icepack.setup --docintfc)