Changes to integrate t-route master branch

[mattw-nws]

Routing_Py_Adapter now attempts to run the master branch main_v04 method if the legacy ngen_routing module is not found (this is not preference ordering, rather is the easiest way to determine which of the two is installed), so either version is runnable. Notably, however, all automated test code now tests the master branch and not the legacy ngen branch!

Additions

• Code to run the new master branch main_v04 method in t-route
• A test case to use on with the master branch

Removals

• Unit test for legacy ngen branch version of t-route... while still possible to use, it is no longer integration tested!

Changes

Testing

1. Automated tests passing
2. Included somewhat complex test case in data/gauge_01073000, which successfully produces output.

Screenshots

Notes

Todos

• Eventually, the legacy ngen branch support should be removed.

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)
• Code follows project standards (link if applicable)
• Passes all existing automated tests
• 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
• Project documentation has been updated (including the "Unreleased" section of the CHANGELOG)
• Reviewers requested with the Reviewers tool ➡️

Testing checklist (automated report can be put here)

Target Environment support

• Linux

[mattw-nws]

@hellkite500 @donaldwj @robertbartel ... maybe @shorvath-noaa and @kumdonoaa ... Before I fix the last test...

With this PR the ngen code itself can now--for now--work with either the old legacy ngen_main t-route code or the new nwm_routing module. This is particularly for use cases where old configs exist, I'm thinking particularly of the calibration work. The question is, what, if anything, should we do with extern/t-route? Should I bump it to t-route:master? Should I leave it in place for now pointing to the ngen branch (we may or may not want to bump the commit in that branch)? Should I remove it entirely?

My inclination is to encourage the build/installation of t-route separately going forward, especially since ngen will find it as long as it's installed (doesn't have to be in a particular dir, like the other modules in extern...or at least more so). So what to do with extern/t-route?

[hellkite500]

I'm inclined to remove the sub module all together and replace it with a little documentation on pulling and installing the desired branch.

[kumdonoaa]

@mattw-nws Just trying to understand what bumping it to t-route:master means, can you explain what changes would or should be made from the action and what results would come in terms of running t-route. Thanks.

[mattw-nws]

@mattw-nws Just trying to understand what bumping it to t-route:master means

Sure, it's basically only a matter of what version of t-route we are "bundling" (in source form) with ngen, if any... we have previously been including the t-route source as a git submodule inside the ngen repo... the question is whether we should continue doing that and what version if any.

[mattw-nws]

Related question... should we keep the troute integration test in the ngen repo... or make a test for ngen integration in the t-route repo... or other?

[kumdonoaa]

Regarding the troute integration test, I feel that we need a meeting where Sean first demonstrates the progress of implementing BMI to t-route and reservoir modules so that you can give us feedbacks on whether we're on a right track or not. After that, it would be great to show us how it looks like to do the integration test from ngen side and what are required. Would that be a good starting point?

[mattw-nws]

This now needs NOAA-OWP/t-route#606 to merge before the macOS routing test will pass, I think.

[mattw-nws]

Believe that when t-route/#606 is merged, the failing macOS test may fix itself... if not some small change may be required here, but everything is failing that test ATM.

[donaldwj]

Almost completely configuration file changes. We may want to document the changes in the pull request page

[mattw-nws]

Yes the configuration file for t-route is changed very substantially. The documentation probably goes in t-route, however, I'd think.

I also filed #505 on the macOS forkbomb issue.