-
Notifications
You must be signed in to change notification settings - Fork 132
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
move deformation out of loop for B grid only #755
Conversation
Yes, just for C grid. CD isn't working well enough yet to worry about.
I'd prefer the former also. Seems like it ought to be more computationally efficient than having all those big conditionals in the subcycling loop. |
This branch has a conflict with the merge of #756. Please review and update. Thanks. |
Looks like this PR is ready to merge. Sorry I haven't taken care of it before now. Unless anyone says it's not ready, I'll merge in the next day or two. Thanks. |
@apcraig I think it's ready but will let you merge it when you're ready to set up new baselines. |
Just to be sure, I ran a full test suite on cheyenne with 3 compilers, https://github.com/CICE-Consortium/Test-Results/wiki/cice_by_hash_forks#d52704ceef4e003aae692e69424f5773a6bf6980. Everything runs fine and almost all answers change. Some of the box tests and eap/vp tests do not, as expected. The QC results pass for all three compilers vs current main, so that should provide verification that climate is the same,
Will merge now. |
* move deformation out of loop for B grid only * Moved C and CD grid deformations * correct location of bgrid deformation
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 adress issue check whether ice strength is calculated at the correct location. Move to after last subcycle? #739
Till Rasmussen, DMI
ENTER INFORMATION HERE
In order to address issue check whether ice strength is calculated at the correct location. Move to after last subcycle? #739 the call to deformation has been moved outside the subcycling loop. The results are expected not bit for bit however they pass qc test. Whether the original implementation or this is more correct is unclear to me but the deformations are called using the velocities at ksub=ndte instead of ndte-1
2 questions
1/ should I make the same test for c and c grid before this is merged.
2/ should we refactor the code so that it reads
if B grid
do sub cycling
end subcycling
else if C grid
do sub cycling
end subcycling
else if CD grid
do sub cycling
end subcycling
endif
As an alternative to the present
do subcycling
if B
else if C
else if CD
endif
enddo.
I would prefer the first but I will not do it before I have heard your opinion.
The qc test png files are attached below