-
Notifications
You must be signed in to change notification settings - Fork 100
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.
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
Vehicle type model REQUIRE_DATA_FOR_ALL_ALTS
needs to be True
#789
Labels
Bug
Something isn't working/bug f
Comments
Merged
Good find @i-am-sijia. I believe I have a fix (#790) in place to also filter alts_long if alts_wide gets filtered. I believe this means the |
Ok, let's add Lines 448 to 451 in a8e755f
More importantly, users should be warned when the software is dropping vehicle type alternatives. In case someone adds vehicle types (e.g., AVs) in the yaml but forgets to add vehicle types in the vehicle type data. |
Addressed with #790 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Describe the bug
The vehicle type choice model creates alts_wide and alts_long tables, and they need to be in the same length and order, because this line of code assumes that for the final mapping of choices:
activitysim/activitysim/abm/models/vehicle_type_choice.py
Line 426 in a8e755f
However, the section of code below drops alternatives from alts_wide if there are alternatives that do not exist in the input
VEHICLE_TYPE_DATA_FILE
and if the optionalREQUIRE_DATA_FOR_ALL_ALTS
does not get defined or is set to False.activitysim/activitysim/abm/models/vehicle_type_choice.py
Lines 217 to 224 in a8e755f
This will make the alts_wide inconsistent with the alts_long, and make the final mapping of choices incorrect.
The model will still run, but the result will be incorrect.
To Reproduce
Steps to reproduce the behavior:
Expected behavior
The model should fail if there are alternatives that do not exist in the vehicle type data.
REQUIRE_DATA_FOR_ALL_ALTS
should not be optional or and should be True. The IF condition on Line 217 can be replaced by the assert.The text was updated successfully, but these errors were encountered: