Correct indexing of cospOUT

[jshaw35]

Hi all,

The cospOUT DDT seems to be missing correct indexing for when inputs are chunked in the CloudSat quickbeam_column call (lines 1272-1277) and the cosp_diag_warmrain call (lines 1640-1670). I have corrected the indexing errors.

@dustinswales mentioned that there might be an issue in how the MODIS diagnostics are passed to the warm rain diagnostics. I think that adding the (ij:ik) indexing when passing cospOUT fields to cosp_diag_warm as inputs should align the other warm rain diagnostic inputs with cospOUT and fix things.

A simple test using the COSP offline driver showed changes relative to the KGO values when chunking is used. This is expected. There should be no changes when there is no chunking in calls to COSP_SIMULATOR.

Best,
Jonah

[alejandrobodas]

Hi @jshaw35 , I've fixed the CI tests in #88 and #89. Please can you merge the changes into your branch so that this change can be tested? Thanks!

[alejandrobodas]

Hi @jshaw35 please can you merge the latest changes (#90) again? These new changes should produce a new set of known good outputs for all compilers. If it all works as expected then we'll be in position to review this change.

[jshaw35]

Hi @alejandrobodas, I think that I merged in the latest changes. But it looks like the integration failed again. If I am doing something incorrectly just let me know. Happy to continue pulling in from the latest changes.

[dustinswales]

@jshaw35 I think we should expect some answer changes here.
@takmichibata as the original developer, what are your thoughts?

[alejandrobodas]

Hi @jshaw35 . Yes, the tests will fail because your changes modify the results. This makes the review of this change a bit complex, despite the simplicity of the code changes. We will guide you through the process. Basically, this is done in two steps:

1. We need to confirm that the new outputs are good. The table with the summary of changes show that these diagnostics have changed:
   Variable N AvgDiff MinDiff MaxDiff StDev
   ptcloudsatflag0 1326 9.63e-01 -1.0000e+00 1.9000e+01 2.3373e+00
   ptcloudsatflag1 105 2.3810e-01 -1.0000e+00 1.0000e+00 9.7124e-01
   ptcloudsatflag2 157 -2.73e-01 -1.0000e+00 3.0000e+00 9.8667e-01
   ptcloudsatflag3 112 5.3571e-01 -1.0000e+00 1.0000e+00 8.4440e-01
   ptcloudsatflag4 119 2.2637e-01 -1.0000e+00 1.0000e+00 9.6820e-01
   ptcloudsatflag5 146 1.5205e+00 -1.0000e+00 1.9000e+01 3.40e+00
   ptcloudsatflag6 36 1.0000e+00 1.0000e+00 1.0000e+00 0.0000e+00
   ptcloudsatflag7 68 1.0000e+00 1.0000e+00 1.0000e+00 0.0000e+00
   ptcloudsatflag8 24 1.0000e+00 1.0000e+00 1.0000e+00 0.0000e+00
   ptcloudsatflag9 3 1.0000e+00 1.0000e+00 1.0000e+00 0.0000e+00
   cloudsatpia 1028 7.9773e-01 -1.0000e+00 2.5667e+01 2.3142e+00
   npdfcld 283 -4.2403e+27 -3.3333e+29 1.0000e+00 3.6072e+28
   npdfdrz 278 -3.5971e+27 -1.0000e+30 -1.0000e+00 5.9868e+28
   npdfrain 278 -1.0000e+00 -1.0000e+00 -1.0000e+00 0.0000e+00
   ncfodd1 207960 -7.5129e+ -1.0000e+30 1.0000e+00 6.7633e+27
   ncfodd2 207903 -1.1357e+ -1.0000e+30 1.0000e+00 8.9176e+
   ncfodd3 207803 -9.7996e+25 -1.0000e+ -1.0000e+00 8.5951e+27

Please can you add to this PR plots of the diagnostics that have changed? In order to do this, you'll need to download and unpack the outputs generated by one of the compilers, e.g. outputs.gfortran-12.UKMO.tgz. You'll find the files at the bottom of the failed tests:
https://github.com/CFMIP/COSPv2.0/actions/runs/8379994107

The files contain plots of the outputs that have been generated automatically by the integration tests, but unfortunately they don't contain plots of the variables that are affected by this PR, so you'll need to produce the plots offline. It is sufficient to add plots from the outputs of gfortran-12 only.

2. Assuming that the plots look as expected from the code changes, then we'll show you how to modify the tests so that the new outputs are used as reference in the tests. This is easy to do. Once this is done, the tests will pass and we will be able to merge your change.

I hope this helps. Please let us know if you need more information and thanks for contributing to COSP!

[jshaw35]

Hi @alejandrobodas, I've finally gotten around to producing the plots of the diagnostics that changed. I updated plot_test_outputs.py, so plots of the missing variables should now be produced during CI (here I've assumed some bounds on the CFODD_NDBZE and CFODD_NICOD axes).

The updated plots after the recent failed CI are here:
https://github.com/jshaw35/COSPv2.0/actions/runs/8639121555

I don't actually use the diagnostic fields that were affected by the indexing error, so I will leave interpretation of the updated plots to you. Just reviewing them, however, I noticed that the histograms for the ncfodd1,2,3 variables almost always have in-cloud optical depth values in the 0-2 bin.

Best,
Jonah

[takmichibata]

@jshaw35 I think we should expect some answer changes here.
@takmichibata as the original developer, what are your thoughts?

Sorry for the late response. I missed the PR notification.
The changes seem fine to me. Thanks Jonah and Dustin.

[alejandrobodas]

This PR has been stalled for a while. The code changes look fine, but as @jshaw35 mentions above the plots of the fields affected by the indexing don't look right to me. @takmichibata please can you have a look at the plots attached?

[takmichibata]

Thank you for sharing the plots @alejandrobodas @jshaw35.
Variables for the # of non-precipitating, drizzling, and precipitating clouds look fine to me. Only the daytime area can be plotted because the warm-rain diagnostics uses MODIS visible retrievals. Samples are between 0 and 20, the maximum number 20 will be consistent with the subcolumns.
On the other hand, I found it strange that sample numbers of ncfodd* are less than 1, but I can understand if the plots are global average values. I also feel no problem that many samples concentrate 0-2 bin of the ICOD because this is not the total (columnar) optical depth but vertically-sliced layered optical depth (i.e., the 0-2 ICOD bin corresponds to near cloud-top). CFODD will be very noisy when we only use a smaller sampling period or snapshot like the offline KGO test.