-
Notifications
You must be signed in to change notification settings - Fork 68
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
Support for lane based network routing #189
Comments
I have completed a version allowing for lane-based routing and pushed it to a forked repo with version no 23.4_r. This version passes the original tests, allowing for minor modifications in the function inputs due to network inversion (allowing access to the main config to most classes and using links instead of nodes while calculating the least cost paths). I have not implemented it yet on any large-scale network for transit mapping. But that is the next phase. Please let me know if you have any concerns or suggestions. Feel free to clone and test out the code. Thanks. Here is the link for the forked repo: |
Thanks for tackling this issue and improving the package! I had a quick look into your fork but I think a review is best done with a pull request. Feel free to open one when you think the code is ready. Something I noticed while looking through the code: You pass a new config (object or file path), mostly called |
Hi, thanks for the reply. The primary purpose was to take the lane definition file location while creating the InvertedLeastCostPathCalculator class which is already a part of the original config. The network is also a part of it, so I did not get why the network file location was taken as an input in the ptMapperConfigGroup. As the config has the ptMapperConfigGroup as part of it, my final intention was to pass the main config rather than the ptMapperConfigGroup to most classes. |
Ah I see. Wouldn't it be easier to extend the ptMapperConfigGroup with a parameter for the lane definition file? |
@polettif As I am new to modifying MATSim code, I was trying for minimal modifications. As the necessary information is already in the config file, which also contains any additional modules, I was trying to push that inside. I have worked on three different parts: first, adding lane-based routing; second, adding support to extract lane information and lane restrictions from OSM; and finally, I have combined a file to combine the two logics to create a network generation workflow. As I am new in coding, I would appreciate some advice on raising pull requests for specific features. I am planning to create a different branch that is synced to the original and then push each feature one by one. I am sorry for the late response. |
The issue with this approach is that you need to change function arguments throughout the codebase: Wherever a ptMapperConfigGroup config is required you need to add another parameter. You shouldn't really use a general MATSim simulation config file for pt2matsim as pt2matsim's functions only use the PublicTransitMappingConfigGroup. In a way we just use the config class to pass on the mapping configuration. Now, if you want to avoid duplicating config info (I don't know how many parameters are needed for InvertedLeastCostPathCalculator) you could also pass the whole config file, especially while developing. You'd still need to define (and document) which parameters of which modules are used though.
That would be a great addition to pt2matsim. It's quite a task to do though, even for one of those parts.
I'd suggest you tackle these steps:
You can develop each feature on your own git fork. Don't forget adding unit tests. After you've developped a feature you can create a pull request so we can review it and add it to pt2matsim. If the modules are set up correctly you don't really need a new workflow as the upgraded modules fit right within the exiting pt2matsim workflow (by updating some default classes). |
Lanes are pertinent tools to model turn restrictions. Given original MATSim has a lane-based routing and given turns are vital to the routing of transit vehicles, it is only fitting to extend the functionality of lane-based routing in pt2MATSim from the core MATSim.
I have tried to implement such. The problems I faced were:
It is still a work in progress. Please advise if you have any suggestions.
The text was updated successfully, but these errors were encountered: