Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update traversal model unit representation #279

Open
nreinicke opened this issue Jan 28, 2025 · 3 comments · May be fixed by #281
Open

Update traversal model unit representation #279

nreinicke opened this issue Jan 28, 2025 · 3 comments · May be fixed by #281
Assignees
@robfitzgerald

Comments

@nreinicke
Copy link
Collaborator

Our current traversal model unit representation could be a little bit clearer. For example, we might have a config that looks like this:

[traversal]
type = "energy_model"
time_model_speed_unit = "kilometers_per_hour"
grade_table_input_file = "edges-grade-enumerated.txt.gz"
grade_table_grade_unit = "decimal"

time_unit = "minutes"
distance_unit = "miles"

[traversal.time_model]
type = "speed_table"
speed_table_input_file = "edges-posted-speed-enumerated.txt.gz"

speed_unit = "kilometers_per_hour"
distance_unit = "miles"
time_unit = "minutes"

Looking at the traversal.time_model section, it's unclear what the units represent. For example, the speed_unit field is used to indicate what units will be found in the speed_table_input_file but the distance_unit is used as the unit that the time model outputs. We also have the key time_model_speed_unit which could be mistakenly set differently from the speed_unit key with no error thrown.

Let's think about how to make these more clear for the user even if that's just updating their name so that speed_unit might speed_table_speed_unit and time_unit might become output_time_unit.

We might also be able to get the time model speed units directly from the model rather than making the user set it.
@robfitzgerald
Copy link
Collaborator

robfitzgerald commented Feb 4, 2025 •
edited
Loading

after a nice IRL discussion at a white board, we have some new ideas here:

  1. it should be possible to inspect the unit types for a given model
  2. in the logic of a model, we shouldn't have to convert units, it should have already happened outside of the model scope
  3. ... do we need a CostUnit and cost per x Unit to make units explicit in the CostModel?

@robfitzgerald robfitzgerald self-assigned this Feb 4, 2025
@robfitzgerald
Copy link
Collaborator

update: while thinking this through, i'm realizing that it is possible a model might have multiple fields with different variations of a unit type. i'm struggling to think of an example for this, but, at least it is possible for a user to provide state features that look like this:

vec![
  (String::from("a"), StateFeature::Distance(DistanceUnit::Meters, Distance::ZERO),
  (String::from("b"), StateFeature::Distance(DistanceUnit::Miles, Distance::ZERO)
]

if this really is possible, then it no longer makes sense to model.get_distance_unit(), since there are two of these here.

this ends up being a pretty good argument to use base units inside of models.

@robfitzgerald robfitzgerald linked a pull request Feb 5, 2025 that will close this issue
Draft
@robfitzgerald
Copy link
Collaborator

created downstream issues to follow after #281 is completed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@robfitzgerald robfitzgerald

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Rjf/279 simplify unit configurations and conversions NREL/routee-compass
2 participants
@nreinicke @robfitzgerald