NOAA-OWP / t-route

Tree based hydrologic and hydraulic routing
Other
43 stars 48 forks source link

Should existing parquet files cause a complete failure? #787

Closed hellkite500 closed 2 months ago

hellkite500 commented 3 months ago

https://github.com/NOAA-OWP/t-route/blob/5bef6342109fd09db27be731225e7c28e968061b/src/troute-network/troute/AbstractNetwork.py#L754

I have run into this in different scenarios, and typically end up just deleting the parquet files manually and running again when needed. Recently, however, I have found a use case that this isn't sufficient...particularly when the t-route routing is combined with any workflow which may also create parquet files. In this case the generic *.parquet check here causes a failure even though these parquet files are not associated with t-route. Additionally, looking through other parts of the relevant code, why is this check needed? It seems that other parts of the code do similar existence checks and have appropriate logic for handling the situation.

It seems like t-route should be able to simply overwrite the old files and continue with a simulation if asked to. At the very least, can this check be narrowed to check for *NEXOUT.parquet so that files can be cleaned up if needed without affecting other parquet files that may exist in the same location?

hellkite500 commented 3 months ago

Partially addressed by #788. With that PR, a workflow can remove old nexout parquet files itself and leave other parquet files. t-route still won't fully overwrite old files as another check in another part of the code will raise an exception and not remove it.

shorvath-noaa commented 3 months ago

should we just remove this? https://github.com/NOAA-OWP/t-route/blob/1a5ccf243db939c186c0cdfe00c080b25cce5ab0/src/troute-network/troute/AbstractNetwork.py#L943-L946

shorvath-noaa commented 3 months ago

should we just remove this?

https://github.com/NOAA-OWP/t-route/blob/1a5ccf243db939c186c0cdfe00c080b25cce5ab0/src/troute-network/troute/AbstractNetwork.py#L943-L946

remove the if/else condition, that is.