Update to use t-route output in csv form

[hellkite500]

At some point t-route stopped supporting flowveldepth output in hdf5 form. However, it will still output the flowveldepth in a csv file which can be used very similarly. This PR updates routing output to support reading the csv output instead of hdf5.

Also, when multiple runs of t-route are used, a cleanup of the *NEXOUT.parquet files is needed, so that was added here in the update_config step to ensure a clean directory for each calibration iteration.

Only works once NOAA-OWP/t-route#788 is applied, otherwise t-route will still throw an error when run through ngen-cal since the working directory contains other, non-troute specific, parquet files (see NOAA-OWP/t-route#787)

[aaraney]

@shorvath-noaa if its not too much trouble, can you point to the commit or PR that removed support for flowveldepth in hdf5?

[aaraney]

Can you check my understanding, @hellkite500? At some point, t-route stopped outputting flowveldepth in its hdf5 output, however that variable is still present in the csv format. Both the csv and hdf5 outputs will be present in all t-route simulations (that output things) so this is not a breaking change, we are more or less just using a different data source? Just wanting to make sure that this is not breaking backwards compatibility.

[shorvath-noaa]

@shorvath-noaa if its not too much trouble, can you point to the commit or PR that removed support for flowveldepth in hdf5?

That was before my time, but I can search for it if that's helpful?

The latest output method for t-route is netcdf files containing all of the values from the flowveldepth array, plus info about streamflow nudging (DA values used). Would those work for this?

[hellkite500]

Can you check my understanding, @hellkite500? At some point, t-route stopped outputting flowveldepth in its hdf5 output, however that variable is still present in the csv format. Both the csv and hdf5 outputs will be present in all t-route simulations (that output things) so this is not a breaking change, we are more or less just using a different data source? Just wanting to make sure that this is not breaking backwards compatibility.

Previously, the code looked for flowveldepth.h5 and read the model output for evaluation from that file. t-route no longer produces the output in hdf5 form, but it can produce the same table in csv form, so this PR just changes the source of the data it was looking for. The selection of output format is a t-route configuration option and not directly the responsibility of ngen-cal, so I don't think this is a breaking change, per-se.

[hellkite500]

@aaraney @shorvath-noaa I found some missing context...appearently t-route will still output hdf5 files -- if you use main_v02 which in turn uses the v02 output function where this functionality was put in place to replace csv outputs with the binary hdf5:

https://github.com/NOAA-OWP/t-route/blob/1a5ccf243db939c186c0cdfe00c080b25cce5ab0/src/troute-nwm/src/nwm_routing/__main__.py#L845

However, this functionality was never refactored into v04 and its subsequent output mechanics, so it may be that we can revive this functionality with the addition of a new output configuration type/functionality.

I'm tempted to refactor this PR to look for hdf5 and revert to CSV and give users a choice based on their system/requirements. Using hdf5 does require tables which has caused some minor hiccups in the past, but I think has been pretty stable and easy enough to use on most platforms/systems.

Thoughts on supporting either binary or text routing outputs in ngen-cal?

[aaraney]

I'm tempted to refactor this PR to look for hdf5 and revert to CSV and give users a choice based on their system/requirements. Using hdf5 does require tables which has caused some minor hiccups in the past, but I think has been pretty stable and easy enough to use on most platforms/systems.

Thoughts on supporting either binary or text routing outputs in ngen-cal?

If its not a crazy lift, this is my vote. Sounds like a little strategy pattern should get the job done!

[hellkite500]

Note that this PR WILL break the explicit calibration semantics, a subsequent PR is in progress to refactor that strategy and bring it in alignment with these changes.

[hellkite500]

Also probably need an updated example config illustrating the use of the model hooks, but that could be pushed in a subsequent PR...

[aaraney]

In general, i'm really happy with how using the plugin system decouples the system and really decomposes things. I think that is a huge win and as we continue down the road, I know we will thank ourselves for going this sort of route.

I left a few comments that should be pretty straight forward to get though. It might be easiest if we found some time tomorrow to sit down and work through them together. Thanks for spending time to think through this stuff!

[aaraney]

Looks good to me! Thanks for working through this with me, @hellkite500! Lets squash and merge this puppy!