dHBV2 compatibility

[quinnylee]

dHBV2 requires some custom forcings (detailed in #29 )
@leoglonz

Additions

• When dHBV2 is specified as the model, only temperature and precipitation are processed
• dHBV2-specific unit conversion (K to degC)
• PET calculation using Hargreaves equation, includes functions to get the latitude of a given catchment
• CFE/NOM and dHBV2 examples added for testing
• GIFs generated for dHBV2 variables except for PET. The temperature one is pretty useless though, since Celsius values are far outside the bounds of Kelvin, and PET doesn't have any NWM values. The temperature range problem could be fixed, and PET could be plotted just for NGEN, if we think it's important/useful enough.

Changes

• Model type can now be specified in config json file

Testing

1. CFE/NOM and dHBV2 examples tested on macOS and Linux

Todos

• Extract only necessary variables from source forcing files
• Convert precip to mm/s and temperature to degC
• PET computation (requires day of year and latitude)

Notes

This is a copy of PR #37 but merging from a CIROH-UA branch instead of my branch so the tests pass

Checklist

• PR has an informative and human-readable title
• Changes are limited to a single goal (no scope creep)
• Code can be automatically merged (no conflicts)
• Passes all existing automated tests (except the one that fails)
• Any change in functionality is tested
• New functions are documented (with a description, list of inputs, and expected output)
• Placeholder code is flagged / future todos are captured in comments
• Visually tested in supported browsers and devices (see checklist below 👇)
• Project documentation has been updated (including the "Unreleased" section of the CHANGELOG)
• Reviewers requested with the Reviewers tool ➡️

Testing checklist

Target Environment support

• macOS
• Linux

Other

• No linting errors or warnings (in my changes) :)

[JordanLaserGit]

@quinnylee Thanks for this new feature!

Before we get this merged, could you

1. add pytests to test the dhbv2 feature, including a unit test for the hargreaves function? Just some "these inputs should equal these outputs".
   When you've got the command(s) for the pytests, please add them here into the workflows.

• Perhaps adding a new unit test (forcingprocessor_*) workflow document here https://github.com/CIROH-UA/forcingprocessor/tree/main/.github/workflows for testing the options for model_type in the config.
• x86 build test here

  forcingprocessor/.github/workflows/build_test_fp_x86.yaml

  Line 82 in 98cd0b9

  - name: "NOMADS"

• arm build test here

  forcingprocessor/.github/workflows/build_test_fp_arm.yaml

  Line 82 in 98cd0b9

  - name: "NOMADS"

2. comment on the performance at short range (18hrs), VPU scale relative to the current processing (model_type = cfe). This is something I'd like to have in an automated workflow, but it would be nice to have some understanding of how fast new algorithms are. I'd guess dhbv2 processing is much faster due to the few output variables.

3. Update the README with the new model_type option.

[JordanLaserGit]

Can this be modified to allow for a combination of variables? For instance, if we want both cfe and dhbv2 variables in the output ngen forcings file. The motivation for this is storage/processing optimization for the nrds.

[quinnylee]

Yes! A couple changes this will cause (writing this mostly for myself), because dhbv takes precipitation in mm/s and I don't need to be duplicating values:

• dhbv's BMI will need to read precip_rate not P
• NGIAB data preprocessor will need to output the variable named precip_rate not P for dhbv runs

[JordanLaserGit]

This function requires a geopackage. Until now, forcingprocessor could be run using only a weights file as input. This is a bit of a hack/short cut, but the weight generation at vpu scale is worth avoiding. I'm making this comment to reference in an issue that can be dealt with outside of this PR.

[quinnylee]

Alternatively, if the vpu-scale weights file is a big parquet file or something, indexed by catchment, we could just store our latitudes in there

[JordanLaserGit]

Ya, it's just a big parquet. That's a good idea, I can work that logic out in a PR after this one.

[quinnylee]

Performance comparison (not on vpu scale, but on watershed scale). I can rerun on vpu scale tomorrow, just need to find or make a geopackage

CFE/NOM:

Calc weights : 41.28s
Process data : 77.84s
Write data : 1.93s
Collect stats : 0.03s
Write tar : 0.97s
Runtime : 124.58s

dHBV2:

Calc weights : 29.90s
Process data : 80.22s
Write data : 2.14s
Collect stats : 0.02s
Write tar : 1.06s
Runtime : 115.83s

Also, README.md has been updated with the additional needed params

[quinnylee]

Here's the performance comparison on VPU06. I'm guessing that even though dHBV has 3 variables, the procesing time is similar to CFE because of the amount of time it takes to calculate PET and latitudes.

CFE:

Calc weights : 128.95s
Process data : 159.88s
Write data : 3.15s
Collect stats : 0.15s
Write tar : 8.98s
Runtime : 303.72s

dHBV2:

Calc weights : 118.30s
Process data : 184.91s
Write data : 2.84s
Collect stats : 0.04s
Write tar : 2.04s
Runtime : 310.70s

[JordanLaserGit]

Thanks for those performance numbers @quinnylee! Looks good enough to me, I just wanted to see if the run time was of the same magnitude for processing either cfe or hbv variables.

[quinnylee]

Closing so we don't duplicate variable processing for multiple models