[Bug]: Silent bug in `adjust_prs_val_units()` conditional where `if prs_val0:` will be `False` if `prs_val0` is 0
Opened this issue · 1 comments
What happened?
Overview
There is a conditional in adjust_prs_val_units()
that checks if prs_val0:
to perform unit adjustment on the prs axis. This is a silent bug in this conditional where unit adjustment is not being performed when prs_val0 == 0
because 0 == False
in boolean form.
Related code:
e3sm_diags/e3sm_diags/derivations/acme.py
Lines 433 to 446 in 43f5f74
Example Case
There are several derived variables that define prs_low0=0
. For example, notice how CLDLOW_TAU_1.3_9.4_MISR
has a prs_low0=0
below. For the dataset I was working with, this value is never used due to the silent bug. No prs axis adjustments occur and the prs low value from the dataset (-0.001) is used which results in incorrect results.
e3sm_diags/e3sm_diags/derivations/acme.py
Lines 1378 to 1393 in 43f5f74
What did you expect to happen? Are there are possible answers you came across?
The conditional should be updated to if prs_val0 is not None:
since prs_val0
is optional. This logic is implemented correctly on the dev branch, cdat-migration-fy24
.
def adjust_prs_val_units(
prs: "FileAxis", prs_val: float, prs_val0: Optional[float]
) -> float:
"""Adjust the prs_val units based on the prs.id"""
# COSP v2 cosp_pr in units Pa instead of hPa as in v1
# COSP v2 cosp_htmisr in units m instead of km as in v1
adjust_ids = {"cosp_prs": 100, "cosp_htmisr": 1000}
if prs_val0 is not None:
prs_val = prs_val0
if prs.id in adjust_ids.keys() and max(prs.getData()) > 1000:
prs_val = prs_val * adjust_ids[prs.id]
return prs_val
Minimal Complete Verifiable Example (MVCE)
No response
Relevant log output
No response
Anything else we need to know?
How I found it
In PR #794, I found the following variables had large diffs. I stepped through the code on main
and the dev branch and found that main
doesn't perform prs unit adjustment while the dev branch does.
Comparing:
* /global/cfs/cdirs/e3sm/www/cdat-migration-fy24/792-lat-lon/lat_lon/Cloud MISR/MISRCOSP-CLDLOW_TAU1.3_9.4_MISR-ANN-global_ref.nc
* /global/cfs/cdirs/e3sm/www/cdat-migration-fy24/main/lat_lon/Cloud MISR/MISRCOSP-CLDLOW_TAU1.3_9.4_MISR-ANN-global_ref.nc
* var_key: CLDLOW_TAU1.3_9.4_MISR
Not equal to tolerance rtol=1e-05, atol=0
Mismatched elements: 39457 / 64800 (60.9%)
Max absolute difference: 22.411116
Max relative difference: 0.6832267
x: array([[nan, nan, nan, ..., nan, nan, nan],
[nan, nan, nan, ..., nan, nan, nan],
[nan, nan, nan, ..., nan, nan, nan],...
y: array([[nan, nan, nan, ..., nan, nan, nan],
[nan, nan, nan, ..., nan, nan, nan],
[nan, nan, nan, ..., nan, nan, nan],...
Comparing:
* /global/cfs/cdirs/e3sm/www/cdat-migration-fy24/792-lat-lon/lat_lon/Cloud MISR/MISRCOSP-CLDLOW_TAU1.3_9.4_MISR-ANN-global_test.nc
* /global/cfs/cdirs/e3sm/www/cdat-migration-fy24/main/lat_lon/Cloud MISR/MISRCOSP-CLDLOW_TAU1.3_9.4_MISR-ANN-global_test.nc
* var_key: CLDLOW_TAU1.3_9.4_MISR
Not equal to tolerance rtol=1e-05, atol=0
Mismatched elements: 9 / 64800 (0.0139%)
Max absolute difference: 0.0970192
Max relative difference: 0.01244658
x: array([[ 0.056777, 0.056777, 0.056777, ..., 1.274017, 1.274017,
1.274017],
[ 0.207892, 0.207774, 0.207536, ..., 1.675944, 1.676576,...
y: array([[ 0.056777, 0.056777, 0.056777, ..., 1.274017, 1.274017,
1.274017],
[ 0.207892, 0.207774, 0.207536, ..., 1.675944, 1.676576,...
Comparing:
* /global/cfs/cdirs/e3sm/www/cdat-migration-fy24/792-lat-lon/lat_lon/Cloud MISR/MISRCOSP-CLDLOW_TAU1.3_9.4_MISR-JJA-global_ref.nc
* /global/cfs/cdirs/e3sm/www/cdat-migration-fy24/main/lat_lon/Cloud MISR/MISRCOSP-CLDLOW_TAU1.3_9.4_MISR-JJA-global_ref.nc
* var_key: CLDLOW_TAU1.3_9.4_MISR
Not equal to tolerance rtol=1e-05, atol=0
Mismatched elements: 34699 / 64800 (53.5%)
Max absolute difference: 45.429226
Max relative difference: 0.9708206
x: array([[nan, nan, nan, ..., nan, nan, nan],
[nan, nan, nan, ..., nan, nan, nan],
[nan, nan, nan, ..., nan, nan, nan],...
y: array([[nan, nan, nan, ..., nan, nan, nan],
[nan, nan, nan, ..., nan, nan, nan],
[nan, nan, nan, ..., nan, nan, nan],...
------------------------------------------------------------------------------------------------------------------------------------------------
Comparing:
* /global/cfs/cdirs/e3sm/www/cdat-migration-fy24/792-lat-lon/lat_lon/Cloud MISR/MISRCOSP-CLDLOW_TAU1.3_MISR-ANN-global_ref.nc
* /global/cfs/cdirs/e3sm/www/cdat-migration-fy24/main/lat_lon/Cloud MISR/MISRCOSP-CLDLOW_TAU1.3_MISR-ANN-global_ref.nc
* var_key: CLDLOW_TAU1.3_MISR
Not equal to tolerance rtol=1e-05, atol=0
Mismatched elements: 39499 / 64800 (61%)
Max absolute difference: 37.673122
Max relative difference: 0.62295455
x: array([[nan, nan, nan, ..., nan, nan, nan],
[nan, nan, nan, ..., nan, nan, nan],
[nan, nan, nan, ..., nan, nan, nan],...
y: array([[nan, nan, nan, ..., nan, nan, nan],
[nan, nan, nan, ..., nan, nan, nan],
[nan, nan, nan, ..., nan, nan, nan],...
Comparing:
* /global/cfs/cdirs/e3sm/www/cdat-migration-fy24/792-lat-lon/lat_lon/Cloud MISR/MISRCOSP-CLDLOW_TAU1.3_MISR-ANN-global_test.nc
* /global/cfs/cdirs/e3sm/www/cdat-migration-fy24/main/lat_lon/Cloud MISR/MISRCOSP-CLDLOW_TAU1.3_MISR-ANN-global_test.nc
* var_key: CLDLOW_TAU1.3_MISR
Not equal to tolerance rtol=1e-05, atol=0
Mismatched elements: 9 / 64800 (0.0139%)
Max absolute difference: 0.0970192
Max relative difference: 0.00541773
x: array([[5.677656e-02, 5.677656e-02, 5.677656e-02, ..., 1.274017e+00,
1.274017e+00, 1.274017e+00],
[2.078919e-01, 2.077735e-01, 2.075364e-01, ..., 1.675944e+00,...
y: array([[5.677656e-02, 5.677656e-02, 5.677656e-02, ..., 1.274017e+00,
1.274017e+00, 1.274017e+00],
[2.078919e-01, 2.077735e-01, 2.075364e-01, ..., 1.675944e+00,...
Comparing:
* /global/cfs/cdirs/e3sm/www/cdat-migration-fy24/792-lat-lon/lat_lon/Cloud MISR/MISRCOSP-CLDLOW_TAU1.3_MISR-JJA-global_ref.nc
* /global/cfs/cdirs/e3sm/www/cdat-migration-fy24/main/lat_lon/Cloud MISR/MISRCOSP-CLDLOW_TAU1.3_MISR-JJA-global_ref.nc
* var_key: CLDLOW_TAU1.3_MISR
Not equal to tolerance rtol=1e-05, atol=0
Mismatched elements: 35149 / 64800 (54.2%)
Max absolute difference: 67.89603
Max relative difference: 0.9691263
x: array([[nan, nan, nan, ..., nan, nan, nan],
[nan, nan, nan, ..., nan, nan, nan],
[nan, nan, nan, ..., nan, nan, nan],...
y: array([[nan, nan, nan, ..., nan, nan, nan],
[nan, nan, nan, ..., nan, nan, nan],
[nan, nan, nan, ..., nan, nan, nan],...
------------------------------------------------------------------------------------------------------------------------------------------------
Comparing:
* /global/cfs/cdirs/e3sm/www/cdat-migration-fy24/792-lat-lon/lat_lon/Cloud MISR/MISRCOSP-CLDLOW_TAU9.4_MISR-ANN-global_ref.nc
* /global/cfs/cdirs/e3sm/www/cdat-migration-fy24/main/lat_lon/Cloud MISR/MISRCOSP-CLDLOW_TAU9.4_MISR-ANN-global_ref.nc
* var_key: CLDLOW_TAU9.4_MISR
Not equal to tolerance rtol=1e-05, atol=0
Mismatched elements: 39323 / 64800 (60.7%)
Max absolute difference: 31.085188
Max relative difference: 0.96666664
x: array([[nan, nan, nan, ..., nan, nan, nan],
[nan, nan, nan, ..., nan, nan, nan],
[nan, nan, nan, ..., nan, nan, nan],...
y: array([[nan, nan, nan, ..., nan, nan, nan],
[nan, nan, nan, ..., nan, nan, nan],
[nan, nan, nan, ..., nan, nan, nan],...
Comparing:
* /global/cfs/cdirs/e3sm/www/cdat-migration-fy24/792-lat-lon/lat_lon/Cloud MISR/MISRCOSP-CLDLOW_TAU9.4_MISR-JJA-global_ref.nc
* /global/cfs/cdirs/e3sm/www/cdat-migration-fy24/main/lat_lon/Cloud MISR/MISRCOSP-CLDLOW_TAU9.4_MISR-JJA-global_ref.nc
* var_key: CLDLOW_TAU9.4_MISR
Not equal to tolerance rtol=1e-05, atol=0
Mismatched elements: 33486 / 64800 (51.7%)
Max absolute difference: 63.126827
Max relative difference: 1.
x: array([[nan, nan, nan, ..., nan, nan, nan],
[nan, nan, nan, ..., nan, nan, nan],
[nan, nan, nan, ..., nan, nan, nan],...
y: array([[nan, nan, nan, ..., nan, nan, nan],
[nan, nan, nan, ..., nan, nan, nan],
[nan, nan, nan, ..., nan, nan, nan],...
Affects the following derived variables:
e3sm_diags/e3sm_diags/derivations/acme.py
Lines 1362 to 1409 in 43f5f74
Environment
main
and all versions of e3sm_diags
that has cosp_bin_sum()
These are the kinds of issues that can be easy to miss, which is why linters and type checkers (e.g., flake8
and mypy
) are valuable and great to use as preventative tools.