-
Notifications
You must be signed in to change notification settings - Fork 22
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
Correct Utility Scale for Fares + Default Transfer Penalty #125
Comments
Gah! Sorry! Thanks for catching! 🙀 |
@lmz I couldn't find the place in the code where the fares are added to the time using value_of_time as the modifier other than the quoted code above...but it would seem like there would be somewhere else where it does this. |
Thx @lmz ! @bhargavasana please add this to your list of places to edit. [ side note, search functionality in GitHub is not ideal ...it didn't find this code segment when i searched for fare. So if you need to find all the instances of something, don't rely on it! ] |
At some point, it might also be helpful to rename the generalized cost to utility as well. |
I don't understand why github search sucks so much! |
Do these same changes need to be made on the Python calculate_cost side too? I'll take a look, but please keep posting the suspect areas, so we can review. |
What is the correct behavior? This CPP Code sets it based on the weight in the pathweights_ft.txt. |
@danielsclint - the fare should ideally be scaled by the value of time and the IVTT constant, not by a coefficient in the pathweights file |
Right now, the Python side converts fares into value of time. Is this the behavior you are expecting? If so, their should be a check for a pathweight_ft weight on fare to ensure it does not get applied. |
I have tried to update the calculation in the calculate_cost python function. Please see the following commit and let me know if it needs to be modified further. |
@danielsclint - can you please review. |
I added two comments on the check-in directly. |
Ok, 4410852 and subsequent commits should take care of the fares component. |
Did a quick fix for transfer penalties to make the default to 0.1 in Python and CPP code. here: 46814c6 TODO in future Should make it configurable or based on IVT Coefficient. |
The calculations in Fast-Trips are done assuming that it is receiving a utility as a cost, not a pure "generalized cost". Thus, the path weights, when multiplied by the value of the variable should produce "utils" as the scale of the output.
There are a few places in the code where it assumes that the scale is "minutes" and since we don't want our martian rocket ship to miss Mars b/c of incompatible units...we need to scale them. This mostly happens in fares and transfer penalties, but could happen in other places that we haven't found yet.
1. Fares
The costs of the fares need to be scaled by the value of time AND the conversion of time to utility.
https://github.com/BayAreaMetro/fast-trips/blob/701134f0d55fc983f76bef24031b99b3cb199350/src/hyperlink.cpp#L770-L775
❓ any other places?
2. Transfer Penalties
There is also a few places in the code where a default transfer penalty is asserted. These should be scaled to be more on the order of 0.02 rather than 1.
https://github.com/BayAreaMetro/fast-trips/blob/c08367062a9a7812c5003a5cde90e1e881e668aa/src/pathfinder.cpp#L905-L912
https://github.com/BayAreaMetro/fast-trips/blob/c08367062a9a7812c5003a5cde90e1e881e668aa/src/pathfinder.cpp#L1305-L1309
https://github.com/BayAreaMetro/fast-trips/blob/c08367062a9a7812c5003a5cde90e1e881e668aa/src/pathfinder.cpp#L867-L870
This will be done in the feature branch: correct-util-scale
cc: @danielsclint @lmz
The text was updated successfully, but these errors were encountered: