monocongo / climate_indices

Climate indices for drought monitoring
https://monocongo.github.io/climate_indices/
Other
353 stars 167 forks source link

Palmers does not work in CLI out of the box. #567

Open mvdebolskiy opened 3 weeks ago

mvdebolskiy commented 3 weeks ago

Describe the bug CLI process_climate_indices --index all fails on palmers.

To Reproduce Steps to reproduce the behavior:

  1. pip install climate_indices
  2. git clone https://github.com/monocongo/example_climate_indices
  3. cd example_climate_indices/example
  4. process_climate_indices --index pnp --periodicity monthly --scales 3 6 --netcdf_precip ./input/nclimdiv.nc --netcdf_temp ./input/nclimdiv.nc --netcdf_awc ./input/nclimdiv.nc --output_file_base ./output/nclimdiv --var_name_precip prcp --var_name_temp tavg --var_name_awc awc --calibration_start_year 1951 --calibration_end_year 2010
  5. Get ValueError: Unsupported index: palmers

Expected behavior This should compute ALL the indices.

Desktop (please complete the following information):

Additional context Also,

Traceback (most recent call last):
  File "/home/matveyd/miniforge3/envs/labs/lib/python3.11/site-packages/climate_indices/__main__.py", line 1776, in process_climate_indices
    _compute_write_index(kwrgs)
  File "/home/matveyd/miniforge3/envs/labs/lib/python3.11/site-packages/climate_indices/__main__.py", line 864, in _compute_write_index
    _parallel_process(
  File "/home/matveyd/miniforge3/envs/labs/lib/python3.11/site-packages/climate_indices/__main__.py", line 1250, in _parallel_process
    raise ValueError("Unsupported index: {index}".format(index=index))
ValueError: Unsupported index: palmers

Seems like palmers is not added as a case in _parallel_process. Is this intended behavior?

monocongo commented 3 weeks ago

Yes, the Palmer indices were included in the original version of this package but they never really passed muster and as such I decided to "deprecate" that index. Someone came along later and I think used ChatGPT to convert the original/ancient Fortran from NOAA into Python. Maybe they didn't update the main processing script to include the Palmers index option again? In any event we need to fix this either by updating the documentation to spell out how this is an experimental implementation (i.e. no proper scientific vetting that I'm aware of) or we just remove the Palmers from the main documentation and instead make a separate section outlining how the Palmers can be used outside of the main processing script. I'd prefer to not provide the Palmers in the main processing script until someone qualified has shown that what we have in play here is a valid implementation. I tried for years at NOAA to get that sort of vetting but the only person who had any chance of doing that was covered up in other work. This new implementation suffers from the same lack of validation, and I'm not a climate scientist so I can't do that myself.

mvdebolskiy commented 3 weeks ago

Can I look at the Fortran code somewhere? I might be able to help, if I have time. For now, just exlude it from the index list? And update documentation?

monocongo commented 2 weeks ago

I am having a hard time finding it myself, but here's a copy of some code I found that seems to be the Fortran I used as the basis of my Python re-write. pdinew.f.gz

What code does NOAA use to produce their PDSI datasets? Is it the most faithful/valid implementation of the algorithms outlined in Palmer's 1965 paper? If not then is there a way to update the code via open source? How does NOAA's implementation compare to others used in the field (such as the one used for ClimateEngine)? Nobody seems to be eager to show their work, otherwise it'd be a lot easier to find. (Sorry for the rant, old grudges die hard!)