Route choice #492
Route choice #492
Conversation
The current method of skimming for A* produces out of bound accesses for A*. This occurs when it reconsiders more nodes than are in the network. The `reached_view` array is primarily for skimming, as such I've disabled skimming for A*. Skimming for early exit Dijkstra's should also be reconsidered.
|
Based on a rudimentary profiling I'm not sure further optimisation here via custom data structures is wise. Path finding, as expected, is taking the vast majority of the runtime. Tomorrow I'll have some graphs and proper benchmarks. |
|
From todays bug fixes and performance improvements the runtime when generating 1000 unique routes over the densest part of the Arkansas model takes on average 2.1s single threaded on my Ryzen 5600x. On average these routes are 352.108 links long. BFSLE reached a depth of 4. More profiling indicates no real change in where the time is spent. Path finding is still by far the slowest portion of this method. |
|
I'm marking this as ready to review as I think it's functionally complete. Tests to come tomorrow. |
|
Looking at the annotated Cython output doesn't look great. Unfortunately not much can be done without writing custom cdef extern's for the stdlib and lying to the Cython compiler that these functions don't throw exceptions to prevent it from attempting to acquire the gil. However, the gcc docs assure me that the overhead of the cpp exceptions themselves, particularly the try-catch blocks, is about 7% and and that gcc handles these in an efficient manner. So if an exception is never thrown, the gil is never acquired, and the overhead of the try-catch blocks is negligible. If something ever does throw an exception anyway we have bigger problems.
|
Graphs should also been marked as seen regardless of if the destination is reachable
No need for both methods
|
After review I plan to merge this into the new feature branch |
* new matrix API * suffixes --------- Co-authored-by: pveigadecamargo <pveigadecamargo@anl.gov>
* linting * Fixes linting packages * Fixes linting packages * . * . * . --------- Co-authored-by: pveigadecamargo <pveigadecamargo@anl.gov>
* Update GMNS urls * Linting
…equilibraE#495) * . * . * . * black * . * Cleans non-needed nodes * . * Updates checkout action to V4 as per deprecation of V3 * Remove indentation and if block --------- Co-authored-by: pveigadecamargo <pveigadecamargo@anl.gov> Co-authored-by: Jake-Moss <jake.moss@uqconnect.edu.au>
* Stable until iter 4 * Add multi-iteration test over all algorithms * Fix mismatched results Overall the root of the issue was that the select link wasn't taking into account the previous step directions in it's calculations. In principle, the patch that modified the select to do just that should have worked, however, the matrix that stored the previous step directions was allocated on the wrong object and, while the object lived the entire duration of the assignment, the matrix was being zeroed at each iteration. I'd overlooked this as it did improve the stability up to 4 iterations, but Jan pointed out to me that the beta's being used were 1.0 (and 0.0 respectively) up until the 5th iteration where the step direction truly started to matter. Another two bugs I found are patched in this commit. 1. The aon object was being leaked outside of the traffic class for loop, this meant that the last traffic class was being used for all select links analysis'. The aon objects are now stored and reused, this may increase memory usage during the assignment as GC can no longer clean the memory allocated by these objects up at the end of every AON assignment, instead it will at the end of all assignments. 2. As noted in the comment `pre_previous_step_direction` was being used in place of `previous_step_direction`. * Silence some warnings * Make black and ruff agree * typo * Rename pre_previous_step_direction to temp_step_direction_for_copy --------- Co-authored-by: pveigadecamargo <pveigadecamargo@anl.gov>
* Move graph_building to its own module. Improve compile times * Add dead end removal * Store dead end links for inspection * Add decorators * Fix * Add compressed graph tests and new data * Small graph compression docs * ruff format * black format as well --------- Co-authored-by: pveigadecamargo <pveigadecamargo@anl.gov>
…te_choice # Conflicts: # setup.py
|
@janzill , Can you merge this into the feature branch in case you are happy with the BFS-LE? |
@Jake-Moss , can you create this same map but using Dijkstra instead? Just for documentation purposes |
Both Dijkstra's and A* are producing the exact same results, this is likely because the heuristic uses is consistent. However I have added an option to switch between A* and Dijkstra's. Changing the heuristic used isn't current possible with some hacks due to redefinitions errors caused by the path finding module being |
|
I've opened a dummy PR containing some scratch work for saving route choice results while this PR is in review, once it's merged I'll move that PR over to the main repo. Jake-Moss#1 |
This pr aims to implemented the BFS-LE algorithm as described in Rieser-Schüssler, Balmer, and Axhausen,
'Route Choice Sets for Very High-Resolution Data'.
Further notes and such are included at the top of the
bfs_le.pyxfile.The method is implemented as a Cython extension with an extension class.
This is an incredibly rough implementation. It makes heavy use of libcpp data structures and blatantly makes inefficient copies and allocations.
Todo:
Implemented custom trie data structures for more efficient route and removed link set storage (requires more thought currently)ParalleliseI believe this method should be parallelised at a higher level, it has just enough moving parts and a clear parallel implementation isn't obvious to me.bfs_le.pyxisn't a great nameIn testing I found an issue with the A* implementation, because it most revisit nodes if it believes it over estimated them but hasn't found the destination yet, the
reached_firstarray may be overflown. I've removed this until if/when I can revisit this to implement proper skimming for A*.