MISR outputs are not properly set to fillvalues for night columns about cospv2.0 HOT 16 CLOSED

On closer inspection, it looks like changes are needed to MISR_SUBCOLUMN too, where only the dist_model_layertops variable is set to fillvalue for night columns:

    ! Fill dark scenes 
    do j=1,numMISRHgtBins
       where(sunlit .ne. 1) dist_model_layertops(1:npoints,j) = R_UNDEF
    enddo

Since this routine also returns box_MISR_ztop and tauOUT, it seems like these should be properly masked if nighttime as well, unless the desired behavior is to handle this masking at a higher level in the code, in which case the masking of dist_model_layertops could be omitted here for consistency.

from cospv2.0.

@brhillman @RobertPincus
There are a couple thing to untangle here.
Firstly, the night-time masking of MISR_mean_ztop is clearly incorrect, at least to me.
MISR_mean_ztop(npoints) = R_UNDEF
should be replaced by
MISR_mean_ztop(j) = R_UNDEF
But... I looked at the COSP1 code, and this indexing issue is present there as well, see line 465 of https://github.com/CFMIP/COSPv1/blob/master/MISR_simulator/MISR_simulator.f.
It surely looks like a bug, but we should check with Roj. I don't have his gitHub info, but we should pull him just to confirm.

Secondly, regarding box_MISR_ztop and tauOUT. Both of these are treated the same in COSP1 as in COSP2. The only difference is that these fields are output from misr_subcolumn() and used as inputs later to misr_column(), whereas this was all encompassed in a single routine misr_simulator() in COSP1.

I propose we ask Roj if he agrees with you proposed changes. Then would you be kind enough to open a PR with your proposed changes?

from cospv2.0.

from cospv2.0.

I'll gladly put together the PR, once we have the desired behavior straight (i.e., at which level we want the masking to be done).

from cospv2.0.

from cospv2.0.

I'll gladly put together the PR, once we have the desired behavior straight (i.e., at which level we want the masking to be done).
That’s what I’m asking @dustinswales to explain.

@RobertPincus
In COSP2, both tauOUT and box_MISR_ztop are local variables in cosp_simulator(), whereas in COSP1 they were local variables within MISR_simulator(). They weren't masked in COSP1, so I didn't mask them in COSP2. I don't know if they need masking, I defer to Roj. They aren't part of the output, so I don't think they need be touched.

The only change necessary is in misr_column(), Ben's first suggested correction.

from cospv2.0.

@dustinswales Fair enough, in the current setup, but would this hold if some decided to use the MISR optical depths and cloud top heights in a different aggregation? Seem like there's an argument for masking as low down as possible?

from cospv2.0.

@brhillman
I view the masking/aggregation as part of the retrieval handled in misr_column(). The only masking being done is for nighttime points, the only aggregation is over cloudy subcolumns. If we do the masking higher up the code it won't have any impact on the aggregation in the column simulator later on.

The question I have is why are we calling the MISR_subcolumn() simulator for all columns if we are only using the daylit points in the statistics ? Seems like a waste, we could only process the sunlit points, compute the same stats, maybe save some time?

from cospv2.0.

Hey @dustinswales yeah I agree masking makes sense at the low-level (misr_column). To answer your question about useless calculations, right now misr_subcolumn loops over both columns and subcolumns. So as the code currently stands the easiest thing to do would be to add a logical check at each column to see if the column is night, and if so then bypass the calculations and instead set the column to R_UNDEF (right now it calculates for each column, and then fills after the fact). The other option would be to take the loop over columns out of misr_subcolumn, so inputs would only have a subcolumn dimension and no column dimension, and instead handle the loop over columns at the higher misr_column subroutine, only calling misr_subcolumn for sunlit columns. I don't think there will be a performance difference either way between these options (same amount of work) so I think it's up to you how you want the code structured.

from cospv2.0.

@brhillman
Both of your suggestions would work, personally I prefer your first suggestion of adding a logical check in the MISR simulator code, similar to the ISCCP way, instead of the MODIS way (there's a bunch of extra code in cosp_simulator() to support the masking for MODIS nighttime scenes which may be better suited within the simulator)

from cospv2.0.

@dustinswales @brhillman I still can't wrap my head around apply the night-time filter at the column rather than the sub-column level, since there's no way for any passive instrument including MISR to make any retrievals at night. Is it just technically easier at this stage?

@dustinswales Should we open another issue in clean up the MODIS code to do better night-time filtering?

from cospv2.0.

@RobertPincus
I propose we add masking over nighttime points in misr_subcolumn(), and add this to the simulator code (ISCCP-like). Just a simple logical switch that doesn't process subcolumn dark scenes. We can assign dark columns to fill at this point, but it won't matter as we use the sunlit mask later for the column aggregation. Of course there are other column fields that should be set to fill (Ben's OG post and suggestion)

Also. Yes let's open a corresponding PR to clean up the MODIS code to also handle the masking internally. I can do this.

from cospv2.0.

@dustinswales I like this idea. @brhillman, it's clear what's being proposed, and you can make a PR?

from cospv2.0.

@brhillman
Ha. Now I remember the reason for this confusing piece of code you identified: #31
It's a hack for the MODIS simulator not internally masking the nighttime columns.

from cospv2.0.

@brhillman No waving my dirty laundry around in public, now. But yes...

It's great having you go through the code. You find all the warts.

from cospv2.0.

@RobertPincus @dustinswales yup I think it's clear, I'll put it together tomorrow.

from cospv2.0.