Make RRTMGP thread-safe

[brhillman]

Make RRTMGP radiation code thread-safe. This fixes a long-standing bug in RRTMGP that prevented us from running the code in threaded configurations. The fix involves a very small change in the RRTMGP interface (in radiation.F90), and similarly small changes in the external RRTMGP repo. In order to pull in the external changes, .gitmodules is updated to point to the "develop" branch of the external RRTMGP repo, and the submodule is updated to point to the latest commit. Changes are BFB for RRTMGP configurations (and should be for non-RRTMGP configurations as well since no code is touched outside of the RRTMGP except .gitmodules).

[brhillman]

Thanks @worleyph for finding these fixes!

[worleyph]

@brhillman and @mt5555 , was the decision then NOT to eliminate the initialization declaration for

 ! Clear-sky heating rates are not on the physics buffer, and we have no
  ! reason to put them there, so declare these are regular arrays here
  real(r8) :: qrsc(pcols,pver) = 0._r8
  real(r8) :: qrlc(pcols,pver) = 0._r8

Even though it works fine this way, there was some concern that it shouldn't :-), or was dangerous if only because it might lead to duplicating the style in a situation that would not be innocuous?

[brhillman]

Oh sorry, I can change that. These don't affect the model simulation, which is why they won't appear to change anything.

[worleyph]

sorry to be a pest, but do you now need to add

  qrsc(:,:) = 0._r8
  qrlc(:,:) = 0._r8

at the beginning of the routine? Or was the initialization not actually needed?

[brhillman]

It shouldn't be needed, but I can add it for safety. Only reason I can see to add it is that these get populated to make outfld calls, but only up to nday/ncol, so there could be garbage in the columns > ncol but < pcols. I'll add an initialization.

[brhillman]

Look better? Running tests now as a sanity check, but it shouldn't change anything...(said that before)

[worleyph]

What does the call to pbuf_get_field do? By the name, would have guessed that it reads data from the physics buffer into the local array. You put the initialization to zero after the call to pbuf_get_field?

[brhillman]

Careful...we are initializing qrsc and qrlc (note the postfix "c" to indicate clear-sky), which are not on the physics buffer. The above call to pbuf_get_field is associating the pointers to qrs and qrl on the physics buffer (note the absence of the "c" for clear-sky). So these are unrelated.

But, to answer your question, pbuf_get_field associates pointer variables to data in the physics buffer. So in general, yes we would want to call pbuf_get_field before initializing the data, because until we associate the pointers, we have no idea where in memory they point to.

[worleyph]

Thanks!

[worleyph]

The code changes that I can see (the ones not in the external repository) look good. Based on earlier discussions, I trust that the appropriate changes were made in mo_gas_optics_rrtmgp.F90 .

[worleyph]

Thanks @worleyph for finding these fixes!

Glad that I could help. Wish that I had remembered the issue with fortran initializations in declarations - this has come up before and I had just spaced it - I would have been more confident about proposing the solution.

[brhillman]

Pinging @singhbalwinder

[singhbalwinder]

Thanks for the reminder. The changes look good to me.

[brhillman]

Thanks @singhbalwinder !

[rljacob]

re-assigning to @wlin7 to balance the integrator work load.

[wlin7]

Merged to next.

[wlin7]

Merged to master.

[brhillman]

Thanks @wlin7!