matsim-vsp / parallel_qsim_rust

Parallel and distributed implementation of the Qsim in Rust
8 stars 0 forks source link

Refactor Network Converter #79

Closed Janekdererste closed 1 year ago

Janekdererste commented 1 year ago

The network converter takes ownership of the IONetwork and mutates it. It re-orders the nodes. I think this is not what should happen. Also, the way of which links are associated with nodes is not very effective. Maybe switch to taking a (unmutable) ref to the Simulation network? Once Ids get implemented the ordering of nodes could be done according to id index.

paulheinr commented 1 year ago

I have already thought about that. The TODO comment in the constructor of the TravelTimesCollectingRoadRouter refers exactly to this issue. https://github.com/Janekdererste/rust_q_sim/blob/50cb8e9af62a866e4bf3c9a5ed2d35fdef6460df/src/simulation/routing/travel_times_collecting_road_router.rs#LL73C9-L73C9 I was using the IoNetwork at the very beginning of my routing tests. When I realized, that the simulation network would be better, other things had a higher priority. But you're right, it should be done.