search

nna-rivers / CTSM

CTSM Fork for NNA-Rivers Project
http://www.cesm.ucar.edu/models/cesm2.0/land/
Other
0 stars 2 forks source link

fix OpenMP errors #14

Open apcraig opened 1 year ago

apcraig commented 1 year ago

Description of changes

These modifications were needed to allow CTSM to compile with OpenMP. I think the changes are all part of the new HillSlope code. I guess this new code wasn't tested with OpenMP? Can we confirm these changes are correct, has the HillSlope code been updated and fixed separately?

YifanCheng commented 1 year ago

Hi Tony, It seems that you only changed some comments in the code. Or is that the syntax for OpenMP? Sorry that I am not familiar with OpenMP. Yifan

YifanCheng commented 1 year ago

Description of changes

These modifications were needed to allow CTSM to compile with OpenMP. I think the changes are all part of the new HillSlope code. I guess this new code wasn't tested with OpenMP? Can we confirm these changes are correct, has the HillSlope code been updated and fixed separately?

I have to double check with Sean whether this code is tested with OpenMP. So far, I have not touched the structure of the Hillslope hydrology code, except for enabling spatially distributed parameters and reading them from the surface dataset. Will this affect OpenMP in anyway? Also I haven't touched the Hillslope code after I merged it into CTSM-PPE branch.

apcraig commented 1 year ago

These are OpenMP directives and tell the compiler about the OpenMP threading. These are important and are used only when OpenMP is turned on.

apcraig commented 1 year ago

Do we know if there is a more recent version of the HillSlope code that we could compare to? Maybe they have encountered similar issues and have fixed and validated the OpenMP. I can confirm my changes allow the code to build and run. And I will do exact restart tests which might hint at reproducibility issues, but OpenMP can be tricky to validate as errors can show up only intermittently when they exist. So there usually needs to be a fairly robust testing process. I guess I wouldn't have these concerns except that there were bugs in the OpenMP directives in the latest code so that worries me a bit.