NCAR / icar

The Intermediate Complexity Atmospheric Research model (ICAR)
MIT License
72 stars 53 forks source link

bugfix: wind 3 physics n-cores differences #163

Closed scrasmussen closed 1 year ago

scrasmussen commented 1 year ago

TYPE: bugfix

KEYWORDS: windfields, halo exchange, boundary conditions

SOURCE: Soren Rasmussen, NCAR

DESCRIPTION OF CHANGES: The halo exchange of the u and v variables doesn't communicate the diagonal values. These are needed for the windfield calculations to be performed correctly. Attempts were made to do a second halo exchange with just the single corner column but there is a bug in either Gfortran 12.1.0 or OpenCoarrays 2.10.1 that causes the A(n,:,:)[neighbor] assignment to occur incorrectly.

TESTS CONDUCTED: Commented out smooth_array calls here and here and then performed runs with physics mp=5 and wind=3. The output of runs with 32 cores and 36 were compared. Without smooth_array the output of u and v are identical after init and after running the model for an hour.

Here is the effect of the two smooth_array lines:

effect_of_smoothing-dev

Here is the effect that the uv_exchange commit fixes:

upstream_t0l4_nosmooth_withSINCOSALPHAnml

Here are the effects on performance, the num iterations column is the number of wind iterations. 1x is with one halo exchange and 2x is with the two halo exchanges that fixes the results. total have the timings fro the 2x halo exchange runs:

image

NOTES: These tests were done with 100 wind iterations which increases the effects of the bug. Also there are issues that will show up in wind=3 on the main branch since it doesn't have the sintheta and costheta fix that is in the develop branch. Also github isn't showing the changes well, I added a do-loop and indented the code but github doesn't show those differences succinctly.

Checklist

Merging the PR depends on following checklist being completed. Add X between each of the square brackets if they are completed in the PR itself. If a bullet is not relevant to you, please comment on why below the bullet.

scrasmussen commented 1 year ago

Updated plots and tables to more clearly present the data.

The difference after initialization from running 32 vs 36 np with smoothing turned off. This shows the difference that this PR is fixing.

wind3_1haloexchange

A table of the time effect of the PR fix. The table shows 64 and 72 np timings, showing the original code with one halo exchange, and the PR code with two halo exchanges. This is running for two hours of the model time

Screen Shot 2023-04-27 at 1 23 03 PM

Slightly separate issue, this shows the difference introduced by those two smoothing functions at level 4 and level 22. The difference increase the higher the level.

smoothing_effect_level4 smoothing_effect_level22