pgRouting / pgrouting

Repository contains pgRouting library. Development branch is "develop", stable branch is "master"
https://pgrouting.org
GNU General Public License v2.0
1.17k stars 368 forks source link

astar_boost_wrapper reverse costs s_x, s_y, t_x, t_y #359

Closed nbrinckm closed 8 years ago

nbrinckm commented 9 years ago

Hey folks,

momently I try to write a route search with some constraints (energy) and I want to integrate it in a postgresql database. However, looking to the source code of astar_boost_wrapper I'm a bit confused on the section on adding the reversed edge to the graph (momently line 170 - 179). I can see that the target and source ids are switched but not the x and y coordinates. I'm not really sure, but I would switch them too, so:

graph_add_edge<graph_t, edge_descriptor>(graph, edges[j].id, edges[j].target, edges[j].source, cost, edges[j].t_x, edges[j].t_y, edges[j].s_x, edges[j].s_y); }

instead of the old: graph_add_edge<graph_t, edge_descriptor>(graph, edges[j].id, edges[j].target, edges[j].source, cost, edges[j].s_x, edges[j].s_y, edges[j].t_x, edges[j].t_y); }

Or is it right in the way it is there already? Does I miss an important point?

Best regards Nils

cvvergara commented 9 years ago

@nbrinckm Unfortunately you are not missing a point, (except that the bug is more deeper than that....) there is a generalized bug that was noticed recently, and for branch develop_2_1_0 has been fixed for 3 functions pgr_dijkstra pgr_drivingdistance and pgr_ksp. (they share the same underlying sql signature) I didn't have time to write the necessary code to read x and y values in the underlying sql, and also I did not include x & y values in the generalized directed and undirected graphs. you might want to look at proposal for 2_1_0 ( https://docs.google.com/document/d/1lgBNr9I-otJmMJMeYyRTxi51VxvX9b6yaRWBT2p_ip4 ) where the errors are mentioned. Look at the code under src/common/src and src/dijkstra (src and sql) to have a look on how it was fixed, maybe if you mimic/use that and eventually make a pull request, we will welcome it.

nbrinckm commented 9 years ago

I created a pull request with the fix for this bug. For adapting the way your dijkstra code works, I think I need some additional time. But I wanted to have this bug fixed now.

Best regards Nils

cvvergara commented 9 years ago

@nbrinckm based on the examples here: http://docs.pgrouting.org/2.0/en/src/astar/doc/index.html#pgr-astar What would the new results be? you can modify this file to update the changes in the results if any? https://github.com/pgRouting/pgrouting/blob/develop/src/astar/doc/index.rst and make a new pull request, thanks

nbrinckm commented 9 years ago

Ok, the results for the first example changed, the second example is ok. I created another pull request with the update for the index.rst file and the astar-any.rest file in the test folder.

cvvergara commented 9 years ago

@nbrinckm Thanks for your modifications, those are included in the next release 2.1.0. I will leave this open issue for milestone for 2.2 based on the comment I made before. But at least now things look much much better for Astar. Vicky

cvvergara commented 8 years ago

AStar has being rewritten for version 2.3 If this issue persists, open a new issue