search

NorESMhub / CAM

Community Atmosphere Model including CAM6-Nor branches
1 stars 20 forks source link

Fixed test related to freezing of rain to produce ice if mean rain si… #30

Closed j34ni closed 3 years ago

j34ni commented 3 years ago

The Fortran standard does not specify the sequence in which compound logical expressions are evaluated in an if instruction.

When two conditions are combined and tested using .AND. as in:

       if (lamr(i,k) > qsmall .AND. 1._r8/lamr(i,k) < Dcs) then
          mnuccri(i,k)=mnuccr(i,k)
          nnuccri(i,k)=nnuccr(i,k)
          mnuccr(i,k)=0._r8
          nnuccr(i,k)=0._r8
       end if​

(from https://github.com/NorESMhub/CAM/blob/cam_cesm2_1_rel_05-Nor_v1.0.2/src/NorESM/micro_mg2_0.F90 at line 1736)

it is wrong to assume that the compiler has to treat them from left to right and stop the evaluation if the first condition becomes true: the second condition might be evaluated first which produces an erroneous arithmetic operation (floating-point exception) when lamr(i,k)=0:

The workaround is to use two consecutive if instructions:

       if (lamr(i,k) > qsmall) then 
               if (1._r8/lamr(i,k) < Dcs) then
                       mnuccri(i,k)=mnuccr(i,k)
                       nnuccri(i,k)=nnuccr(i,k)
                       mnuccr(i,k)=0._r8
                       nnuccr(i,k)=0._r8
               end if
       end if​​
j34ni commented 3 years ago

@camnor-core: Anybody following this up?

MichaelSchulzMETNO commented 3 years ago

I think this can be just merged. @DirkOlivie ?

DirkOlivie commented 3 years ago

I am a little bit scared of changing things in the branches which are used for noresm2 and keyClim.

However, if it is a change that does not change existing results, then it is fine for me.

Another option would be to put the changes in the branch cam-Nor-dev, the branch which should be used for new developments and is allowed/aimed to have answer-changing modifications.

j34ni commented 3 years ago

Another option would be to branch from a more recent version of CESM, since NCAR have fixed this particular bug now

j34ni commented 3 years ago

Splitting this compound test in two does not change the results: it only prevents the model from crashing randomly !

MichaelSchulzMETNO commented 3 years ago

Hi, I can not see how this code bit can change results. Why is this closed now as pull request?