Closed brhillman closed 2 years ago
Another note on this, it looks like bugs like this could easily slip through the cracks because the input data used in the regression tests do not contain any nighttime points (the min and max of the sunlit variable are both equal to 1). Changing at least one column in these tests to be non-sunlit would improve the test coverage and make sure that code changes do not affect the day/night masking that is built into the code.
@brhillman
Thanks for pointing this out. You are correct, this is an error. Both modis_Optical_Thickness_vs_ReffICE
and modis_Optical_Thickness_vs_ReffLIQ
need to be set to R_UNDEF, analogous to how we treat modis_Optical_Thickness_vs_Cloud_Top_Pressure
.
Would you be willing to open a PR with proposed fix? Also, I love the idea of having a daytime column in the sample data for regression testing. Would you be willing to provide a modified file as part of the PR?
Cheers, Dustin
@dustinswales no problem I'll put it together tomorrow.
This looks like a bug, and I can confirm in the implementation in our model that
modis_Optical_Thickness_vs_ReffICE
andmodis_Optical_Thickness_vs_ReffLIQ
appear to not be masked properly for night columns. They do appear to be set properly for sunlit columns, but lack a corresponding statement to mask the non-sunlit columns (as is correctly done formodis_Optical_Thickness_vs_Cloud_Top_Pressure
on line 1442 incosp.F90
). This probably won't matter for models that redundantly handle this outside of the COSP infrastructure, but does assume that users will need to treat sunlit vs non-sunlit cases explicitly to avoid dealing with the unset non-sunlit columns.