763 msa precip distribution

[msahn]

This is to implement precip distribution metrics into the PMP.

[msahn]

@lee1043 @acordonez Could you help review this pull request when you have time? It is not urgent because this branch should be merged right before our paper is submitted.

I have a question. In the JSON output structure (see below), there is a hierarchical structure for seasons and months. Is this structure working for CMEC as well? or Should I update it as a flat structure?

[acordonez]

@msahn I'll review this by the end of the week. Can I run these metrics with data that is already in the PMP sample data set?

[msahn]

@msahn Can you add your driver script(s) to setup.py here?

Thank you for pointing it out. I have added my driver script to setup.py.

[msahn]

@msahn I'll review this by the end of the week. Can I run these metrics with data that is already in the PMP sample data set?

Thank you. Yes, it works with daily precipitation data. The default reference data for the Perkins score and other metrics that need reference data is set as IMERG as below. So, this should be also changed to your input data for testing. https://github.com/PCMDI/pcmdi_metrics/blob/2ccbf1aa77611f92962a901524f8f745825ab6f2/pcmdi_metrics/precip_distribution/param/precip_distribution_params_IMERG.py#L43

[acordonez]

@lee1043 @acordonez Could you help review this pull request when you have time? It is not urgent because this branch should be merged right before our paper is submitted.

I have a question. In the JSON output structure (see below), there is a hierarchical structure for seasons and months. Is this structure working for CMEC as well? or Should I update it as a flat structure?

@msahn This structure works fine in the CMEC JSONs.

[lee1043]

@msahn I have made some changes to pass code-formatting check from pre-commit.

While I have resolved most of them, followings are still remaining and I need your help to fix them. Could you please take a look and fix them? You will need to do git pull first before you make any change in your branch, to avoid any code conflict.

pcmdi_metrics/precip_distribution/lib/lib_precip_distribution.py:546:5: F841 local variable 'pmax' is assigned to but never used
pcmdi_metrics/precip_distribution/lib/lib_precip_distribution.py:572:9: F841 local variable 'binrend' is assigned to but never used
pcmdi_metrics/precip_distribution/lib/lib_precip_distribution.py:1212:5: F841 local variable 'ar6_ocean' is assigned to but never used
pcmdi_metrics/precip_distribution/lib/lib_precip_distribution.py:1921:5: F841 local variable 'ar6_ocean' is assigned to but never used
pcmdi_metrics/precip_distribution/scripts_pcmdi/parallel_driver_cmip5.py:12:43: F821 undefined name 'modpath'
pcmdi_metrics/precip_distribution/scripts_pcmdi/parallel_driver_cmip5.py:24:47: F821 undefined name 'res'
pcmdi_metrics/precip_distribution/scripts_pcmdi/parallel_driver_cmip5.py:24:80: F821 undefined name 'res'
pcmdi_metrics/precip_distribution/scripts_pcmdi/parallel_driver_cmip6.py:12:43: F821 undefined name 'modpath'
pcmdi_metrics/precip_distribution/scripts_pcmdi/parallel_driver_cmip6.py:24:47: F821 undefined name 'res'
pcmdi_metrics/precip_distribution/scripts_pcmdi/parallel_driver_cmip6.py:24:80: F821 undefined name 'res'

[msahn]

@msahn I have made some changes to pass code-formatting check from pre-commit.

While I have resolved most of them, followings are still remaining and I need your help to fix them. Could you please take a look and fix them? You will need to do git pull first before you make any change in your branch, to avoid any code conflict.

pcmdi_metrics/precip_distribution/lib/lib_precip_distribution.py:546:5: F841 local variable 'pmax' is assigned to but never used
pcmdi_metrics/precip_distribution/lib/lib_precip_distribution.py:572:9: F841 local variable 'binrend' is assigned to but never used
pcmdi_metrics/precip_distribution/lib/lib_precip_distribution.py:1212:5: F841 local variable 'ar6_ocean' is assigned to but never used
pcmdi_metrics/precip_distribution/lib/lib_precip_distribution.py:1921:5: F841 local variable 'ar6_ocean' is assigned to but never used
pcmdi_metrics/precip_distribution/scripts_pcmdi/parallel_driver_cmip5.py:12:43: F821 undefined name 'modpath'
pcmdi_metrics/precip_distribution/scripts_pcmdi/parallel_driver_cmip5.py:24:47: F821 undefined name 'res'
pcmdi_metrics/precip_distribution/scripts_pcmdi/parallel_driver_cmip5.py:24:80: F821 undefined name 'res'
pcmdi_metrics/precip_distribution/scripts_pcmdi/parallel_driver_cmip6.py:12:43: F821 undefined name 'modpath'
pcmdi_metrics/precip_distribution/scripts_pcmdi/parallel_driver_cmip6.py:24:47: F821 undefined name 'res'
pcmdi_metrics/precip_distribution/scripts_pcmdi/parallel_driver_cmip6.py:24:80: F821 undefined name 'res'

Thank you very much for your help fixing the code style and format.

I have removed the unused local variables in "lib_precip_distribution.py" but could not fix the issues in "parallel_driver_cmip5.py" and "parallel_driver_cmip6.py". In the codes, "modpath" and "res" are defined from parameter files, but it seems like the checking algorithm can not recognize it.

[lee1043]

I have removed the unused local variables in "lib_precip_distribution.py" but could not fix the issues in "parallel_driver_cmip5.py" and "parallel_driver_cmip6.py". In the codes, "modpath" and "res" are defined from parameter files, but it seems like the checking algorithm can not recognize it.

@msahn I merged parallel_driver_cmip5.py and parallel_driver_cmip6.py into a single file, parallel_driver_cmip.py, which reads in the parameter file.

python -u parallel_driver_cmip.py -p ../param/precip_distribution_params_cmip5.py

Its usage for cmip5 and cmip6 is briefly documented here. Could you check if this is okay and work for you?

I also wonder if this setup allows running the code for just one or a few models. If so could you explain how?

[msahn]

I have removed the unused local variables in "lib_precip_distribution.py" but could not fix the issues in "parallel_driver_cmip5.py" and "parallel_driver_cmip6.py". In the codes, "modpath" and "res" are defined from parameter files, but it seems like the checking algorithm can not recognize it.

@msahn I merged parallel_driver_cmip5.py and parallel_driver_cmip6.py into a single file, parallel_driver_cmip.py, which reads in the parameter file.

python -u parallel_driver_cmip.py -p ../param/precip_distribution_params_cmip5.py

Its usage for cmip5 and cmip6 is briefly documented here. Could you check if this is okay and work for you?

I also wonder if this setup allows running the code for just one or a few models. If so could you explain how?

Thank you for updating the code to be more efficient. I have tested it, and there was no issue. I also have added functionality in parallel_driver_cmip.py for running the code for one model using --mod option.

[lee1043]

After adding dependencies in dev.yml github action's build test get automatically cancelled every time it was triggered.

@acordonez do you have any idea on this? Should we consider specifying versions of the newly added dependencies?

[lee1043]

Note: Below four dependencies were added in conda-env/dev.yml. I think I can include them as default for now.

• netcdf4=1.6.0
• regionmask=0.9.0
• rasterio=1.2.10
• shapely=1.8.0

However, considering they are only used for precip distribution metric, further discussion will be needed to decide whether we want to keeping them as default or optional.

@acordonez @msahn any comment welcome.

[acordonez]

@lee1043 We'll also need to add those as dependencies in the conda feedstock meta.yaml if we want them to be installed with PMP via conda-forge. I've had a lot of trouble with shapely in particular trying to install it after creating a new environment with other packages in it, so installing these at the same time as the other PMP dependencies is probably the best way to go.

[lee1043]

@lee1043 We'll also need to add those as dependencies in the conda feedstock meta.yaml if we want them to be installed with PMP via conda-forge. I've had a lot of trouble with shapely in particular trying to install it after creating a new environment with other packages in it, so installing these at the same time as the other PMP dependencies is probably the best way to go.

@acordonez good to know! Agreed, thanks for your input!

[acordonez]

@lee1043 @msahn Looking at the build failures...I don't see anything wrong, but I'll keep looking.

[acordonez]

@lee1043 @msahn I'm stumped about this failure. I've tried rerunning the job a couple of times and it happens in the same step each time. I don't see any warnings or messages related to the new dependencies, but maybe there's some additional step we have to take to include them. Do you think we could ask Jason or Tom?

[lee1043]

@acordonez thanks for checking. Same here, I have looked through messages, rerunned build test multiple times with enabling debugging, but it keeps cancels after ~5 min for no obvious region, which is strange. Do you think it is okay we just override it for now...?

[lee1043]

By above 2 commits, I confirmed that the build test cancelling was not influenced by newly added dependencies in the dev.yml. With that it may be okay to override...