search

NOAA-OWP / t-route

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

Update variable names/units in BMI modules #626

Open shorvath-noaa opened 1 year ago

shorvath-noaa commented 1 year ago

We've used internal t-route variables names in our BMI modules, but we should update these to conform to CSDMS standard names.

Current behavior

In bmi_troute.py and bmi_reservoirs.py, variable names are mostly internal t-route names.

We also need to update the dictionary linking the (should be) standard CSDMS names and internal t-route names, along with respective units.

Once both of those are complete, we can change the initialization of variables to be a simple for loop.

Expected behavior

Standard names are generally of the form _object_quantity. For example: channel_bank_water__volume-per-length_flow_rate ("lateral flow" into side of channel)

More examples.

Steps to replicate behavior (include URLs)

1.

Screenshots

shorvath-noaa commented 1 year ago

I believe @kumdonoaa has begun work on this issue.

kumdonoaa commented 1 year ago

I believe @kumdonoaa has begun work on this issue. Yes. I am working on this.

kumdonoaa commented 1 year ago

@AminTorabi-NOAA This is what I am working on updating current variable names according to CSDMS standard names.

In the codes attached here, commented-out names are existing or internal names t-route use and names on the left on each code line are my suggested name. We can modify them as necessary as well.

https://github.com/kumdonoaa/t-route/blob/a4731b94745c0e3425659da15dea315d0caffd1c/src/bmi_DAforcing.py#L80-L86

https://github.com/kumdonoaa/t-route/blob/a4731b94745c0e3425659da15dea315d0caffd1c/src/bmi_DAforcing.py#L110-L118

I think @AminTorabi-NOAA can start with variables used in these files: https://github.com/NOAA-OWP/t-route/blob/master/src/bmi_troute.py https://github.com/NOAA-OWP/t-route/blob/master/src/bmi_reservoirs.py

Changing names should involve updating the same or similar names used in related files, so let's suggest new names here in this Issue first until we all agree on.