Fix precipitation_sum so it does not return zeros

[AnnaKwa]

I found that the total_precipitation field was zero in reservoir training data, even when there was no precipitaiton update in the state update dict:

I traced this issue to this line of the postphysics stepper, which autofills a total precipitation update using the net_moistening returned by the stepper's get_diagnostics method. The problem is that not all postphysics steppers have a net moistening diagnostic, and in the case of the Prescriber it just returns an empty data array. When an empty column moistening input is added to the physics precipitation field in the precipitation_sum function, the sum is nan and the nonnegative precip limiter converts the nans to all zeros.

Effectively this means that for most of the steps in the training runs where a prescriber was used to correct the state, the physics precipitation was not being applied to the land surface.

I added a couple lines to precipitation_sum to check for empty dq2 inputs and convert to zeros if that is the case.

• [x] Tests added

[brianhenn]

I guess it's because we want the diagnostics to be computed at apply time, not compute time, and they depend on the state (weakly) because they are column-integrated quantities that use delp. Mostly just convincing myself that there isn't a way to simplify things to minimize issues like this by eliminating the separate functions, but I'm guessing it's not worth it.

[AnnaKwa]

one thing I can't recall at this point is why we need to maintain a separate get_diagnostics method (which as it happens also dictates the amount of precipitation sent to the land surface model), rather than just using the __call__ routine's returned diagnostics and/or tendencies to get these things.

Edit- whoops, wrote this before seeing your second comment.

I think it's because the postphysics stepper __call__ occurs in the compute_postphysics step but the get_diagnostics is called in the apply_postphysics. It'd definitely simplify things if calling get_diagnostics and adding precip updates was done in the compute step, though I'm sort of scared to do so in case there's a reason for why that was done in the first place and I end up breaking something in the MLStepper case.

[brianhenn]

I don't have an obvious suggestion so approving for now.

My guess is that this had to do with the fact that we previously computed momentum nudging tendencies in postphysics, but then didn't apply them until the next timestep due to the dycore/physics grid issue, so there was an intervening dycore call between computation and application. In that case it really makes sense to use the updated delp to compute the diagnostics, but we no longer do that since Spencer made it possible to set the A-grid winds (all tendencies are applied directly after postphysics computation). Sooo...I guess it is possible that we could merge the compute and apply postphysics steps, where it should be more obvious that the prescriber should produce an array of 0s for net_moistening. But that is a lot of refactoring and only tangential to this PR.

My other suggestion would be to disallow the possibility of the empty net_moistening data array coming from get_diagnostics. Empty data arrays seem like they are likely to cause trouble based on this experience. We could have some common method that steppers call to compute net_moistening that would fill it in with zeros as appropriate. Again outside of this PR though.