Performance improvements for CSLAM (from John Dennis, CISL) #1365
Conversation
|
I am not getting B4B in this test: ERP_D_Ln9.ne30pg3_ne30pg3_mg17.FHISTC_LTso.derecho_intel.cam-outfrq9s (low top FHISTC with CSLAM) @johnmauff: Could these changes be round-off (order of operation changes?) |
| end do | ||
| end do | ||
| if (present(fotherpanel)) then | ||
| fotherpanel (1-nht:nc+nht,1-nht:0 ,1)=fcube(1-nht:nc+nht,1-nht:0 ) | ||
| do halo=1,nhr | ||
| ftmp(:) = fcube(:,halo) |
There was a problem hiding this comment.
I just realized that the copy to ftmp is not needed here.
| ! | ||
| ! fill in "n" on Figure above | ||
| ! | ||
| do halo=1,nhr | ||
| ftmp(:) = fcube(:,halo) |
There was a problem hiding this comment.
The ftmp(:) is not used.
|
Peter,
I would guess that they are round-off changes. I have not check with a
low top model. I only tested this with a high-top model, looking at the
nstep output.
John
…On Mon, Aug 18, 2025 at 8:42 AM Peter Hjort Lauritzen < ***@***.***> wrote:
*PeterHjortLauritzen* left a comment (ESCOMP/CAM#1365)
<#1365 (comment)>
I am not getting B4B in this test:
ERP_D_Ln9.ne30pg3_ne30pg3_mg17.FHISTC_LTso.derecho_intel.cam-outfrq9s
(low top FHISTC with CSLAM)
@johnmauff <https://github.com/johnmauff>: Could these changes be
round-off (order of operation changes?)
—
Reply to this email directly, view it on GitHub
<#1365 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADH7NUU3WXMFA2QRNJ45PRL3OHQ4XAVCNFSM6AAAAACEFIYUR2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTCOJXGIZDINJRGQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
| if(ns==3) then | ||
| dotproduct = DotProduct_3(w,f) | ||
| else | ||
| dotproduct = DotProduct_gen(w,f,ns) | ||
| endif |
There was a problem hiding this comment.
@johnmauff I guess the NBFB answer comes from this function? When ns = 3, the dot product is hard coded for performance optimization, but the truncation error will be different from the general version since they are now computed in a single line.
There was a problem hiding this comment.
I tried the general algorithm but that did not change BFB differences with the baseline.
There was a problem hiding this comment.
@PeterHjortLauritzen I was able to produce BFB result with the baseline after doing the following things:
- Fixed the typo (confirm by @johnmauff ) at line 1797 (https://github.com/ESCOMP/CAM/pull/1365/files#diff-8beef36006cafdecbf26406cf4357fc0797eb6c7316d9c6aa0a2486fade88f25R1797).
- Merged the latest
cam_developmentbranch into thedennis_perf_cslam1branch (currently thedennis_perf_cslam1branch is based on the tag cam6_4_089).
I ran the baroclinic wave test case (FKESSLER) and compared the optimized version against the baseline/trunk. Below is PS at day 10: 
For comparison, here is a pertlim test (in this case perturbing PS by 1E-14): 
The pertlim test produces errors about 100× smaller. It’s unclear whether it matters that the optimized code introduces round-off errors at every time step, whereas the pertlim test only introduces them at initialization. I'll keep looking/thinking ... UPDATE: all tests were due to code bug ... all tests (I am running) are now BFB |
|
@PeterHjortLauritzen I am curious: I thought threading was not supported for the SE dycore (#941). Thus why the ERP test still works here? |
|
@PeterHjortLauritzen I ran the |
| do halo=1,nhr | ||
| ! ftmp(:) = fcube(nc+1-halo,:) ! copy to a temporary |
There was a problem hiding this comment.
@PeterHjortLauritzen @johnmauff I think this line should not be commented out?
There was a problem hiding this comment.
Good catch. I missed that.
There was a problem hiding this comment.
Thanks @sjsprecious ! Now the results make sense ... see plots above (in a couple of minutes)
Don't know; I just took the test from the CAM test list without thinking much about the actual test (correct! Threading is currently broken in the SE dycore). @nusbaume Do you know the answer to @sjsprecious 's question? |
|
All the tests I am running are BFB now! Thanks @johnmauff and @sjsprecious ... @cacraigucar: This PR is ready to go ... |
|
Hi @PeterHjortLauritzen @sjsprecious, an ERP test takes the default thread and task count from the first case and divides it by 2 for the second case if the number is greater than one. You can see that logic in the CIME code here: https://github.com/ESMCI/cime/blob/master/CIME/SystemTests/erp.py#L32 Given that the default configuration for an SE dycore run is one thread but multiple MPI tasks, it ends up adjusting the tasks but not the threads (which is what allows the test to run with this CAM configuration in the first place). Also, my understanding is that the difference between ERS and ERP is that an ERS test won't change the task layout at all and simply checks that the restart run is bit-for-bit, while the ERP test will halve the processor count for the restarted run before checking if the results are bit-for-bit. Anyways, I hope that helps, and thanks again for getting these improvements into CAM! |
Thanks @nusbaume for your clarification. Clearly I misunderstood the ERS and ERP tests before, but now it is clear to me. |
There was a problem hiding this comment.
@PeterHjortLauritzen could you look at line 487 of this file? The denominator is in a form that is inconsistent with other polynomial evaluations. Is this missing a parens? I.e. should it be invtmp = 1.0_r8 / (recons(6,i,j) + spherecentroid(1,i,j))
Closes #1360