Subroutine COSP_OPAQ declares surfelev as optional about cospv2.0 HOT 8 CLOSED

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

from cospv2.0.

@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?

from cospv2.0.

@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?

from cospv2.0.

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.

from cospv2.0.

@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.

from cospv2.0.

@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?

from cospv2.0.

@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?

from cospv2.0.

Closed.
See #64

from cospv2.0.