Subroutine COSP_OPAQ declares surfelev as optional

[brhillman]

Subroutine COSP_OPAQ declares input variable surfelev as optional, but it is used throughout the routine as if it is always present. This causes a segfault when using the Intel compiler if surfelev is not passed to the routine. My guess is that surfelev probably should not be optional at all, but if this is the desired interface then simply wrapping code that uses it in a logical that checks for the presence of surfelev would be sufficient. For example, at line 1429 in lidar_simulator.F90, the following could be used as a fix:

                 if (present(surfelev)) then
                    cldtypemeanzse(ip,1) = cldtypemeanzse(ip,1) + (( vgrid_z(zopac) + vgrid_z(z_top) )/2.) - surfelev(ip)
                    cldtypemeanzse(ip,3) = cldtypemeanzse(ip,3) + ( vgrid_z(zopac) - surfelev(ip) )
                 else
                    cldtypemeanzse(ip,1) = cldtypemeanzse(ip,1) + (( vgrid_z(zopac) + vgrid_z(z_top) )/2.)
                    cldtypemeanzse(ip,3) = cldtypemeanzse(ip,3) + ( vgrid_z(zopac) )
                 end if

and likewise in other spots in the code that use surfelev.

[dustinswales]

@brhillman
Thanks for bringing this to our attention. Certainly looks as surfelev should not be declared as optional.
@rodrigoguzman-lmd Thoughts?

[rodrigoguzman-lmd]

@dustinswales @brhillman
Thanks for your messages, and sorry for the delayed answered. Yes, surfelev should definitively not be declared as optional. Dustin, what do you think if I open a new pull request where I could fix this and the bug concerning the opaque temperature diagnostics?

[rodrigoguzman-lmd]

@dustinswales @brhillman
Actually, it is not the COSP_OPAQ routine which declares surfelev as optional, it is the lidar_column routine which is in the same file (lidar_simulator.F90). So the title of this issue is not appropriate. The lidar_column routine declares surfelev as optional because it is only used by the CALIPSO simulator so far. @dustinswales , what shall we do, leave it as it is for the time being or declare surfelev as not optional in lidar_column and, hence, add this compulsory argument to the other lidar platform calls even if they don't use the surfelev variable so far?

[brhillman]

Oh, apologies, I might have had to add the optional to get it to run, because it's optional in lidar_column but non-optional in cosp_opaq, which is called from lidar_column. So either way this is a problem, because surfelev may not be present in lidar_column but is assumed present (required) in cosp_opaq. But I'll fix the description to be clear about this.

[dustinswales]

@brhillman @rodrigoguzman-lmd @RobertPincus
Sorry for the late response, just now getting caught up on all things COSP.

After going through the code, I vote to keep surfelev as an optional input to lidar_column(). The reasoning is that in cosp.F90, lidar_simulator() is used by three different simulators (Calipso, ATlid, ground-based 532nm lidar), but only the Calipso simulator requires surfelev (to call cosp_opaq(), which it always does). Additionally, surfelev is protected throught lidar_column() with lcalipso flags.
Another option would be to have two interfaces to lidar_column(), but I'm in favor of the former.

[RobertPincus]

@dustinswales Your logic is sound. Does it imply we should see if @brhillman is willing to open a PR with changes to protect against using the variable when it's not provided?

[dustinswales]

@dustinswales Your logic is sound. Does it imply we should see if @brhillman is willing to open a PR with changes to protect against using the variable when it's not provided?

@RobertPincus
The only place we need to add code is to the subroutine cosp_errorCheck(). There are three checks performed, 0) Are all fields allocated for requested simulators? 1) Are provided fields in range? 2) Are input fields sized consistently?
We would need to add logic in all three parts for both the Calipso and Cludsat simulator, as they both rely on surfelev.
@brhillman Would you be willing to open a PR?

[dustinswales]

Closed.
See #64