Skip to content
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

dijkstra_shortest_paths_no_init supports pruning on distances #13

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

wygos
Copy link

@wygos wygos commented Jul 23, 2014

When the distance map is given and the newly discovered distance
is worse than the distance stored in the distance map,
the vertex is not even added to the priority queue.

@Belcourt
Copy link
Member

Belcourt commented Nov 2, 2014

I'm not sure about this. It seems to me that if the distance decreases, line 145 of dijkstra_shortest_path.hpp, then there's a call to dijkstra_queue_update() which certainly appears to update the queue (m_Q). Perhaps I'm not clear on what you're trying to achieve but the priority queue does seem to be updated when the distance decreases. Can you check this again and give more details if you still believe it's broken?

@wygos
Copy link
Author

wygos commented Nov 12, 2014

The main issue is to make dijkstra_shortest_paths_no_init support more efficient.
This is done by pruning the vertices, that are never going to be improved.

Example:

Assume my graph is as follows:

1 --- 2 --- 3 --- 4 --- 5 --- 6

Assume that weights of all edges equals 1, also assume that we run dijkstra_shortest_paths_no_init, starting from vertex number 1 and we provide the following distance map:

1 -> 0
2 -> infinity
3 -> infinity
4 -> 1
5 -> 1
6 -> 1

Vertices 1,2,3 are initialized in the standard way, but vertices are initialized with smaller values.
In this case dijkstra_shortest_paths_no_init supports should never visit vertex number 4 since the corresponding shortest path weight equals 2 > 1.
The vertices 5 and 6 shouldn't be even discovered.

If one'd like to see a natural use case, please see this algorithm:
http://paal.mimuw.edu.pl/do_vv_2kminus1.html
(now, in order to gain performance, the nasty workaround is used in this code)

The second motivation for this change is that dijkstra is NOT BFS. It was impemented this way using not very intuitive vistor patern usage.
Maybe BFS could be seen as special case of dijkstra with appropriate queue, but not the other way around (current version).

@anadon
Copy link
Contributor

anadon commented Aug 31, 2018

I'm helping out with the PR backlog. Looks like you have a code change, no test, and no new examples will be needed. This is very old and I do not know if it is still relevant. Could I get @wygos to chime in? This is to let you know and help me prioritize PR's.

@wygos
Copy link
Author

wygos commented Aug 31, 2018

Hi, I'm happy to hear that your working on it! It is still relevant. In some special cases, the algorithm is inefficient. There was a discussion in the boost mailing list:

https://lists.boost.org/Archives/boost/2015/10/226337.php

and the consensus was, that the patch should be accepted.

Regards,

Piotr

@anadon
Copy link
Contributor

anadon commented Aug 31, 2018

Since this looks more like a feature, I'll try to include it in the second batch of merges. Thanks!

@anadon
Copy link
Contributor

anadon commented Oct 15, 2018

@wygos Can you rebase your PR on devel?

@wygos
Copy link
Author

wygos commented Oct 22, 2018

@wygos Can you rebase your PR on devel?

DONE. Note that github shows incorrect diffs but after fetching changes everything is OK.

@anadon
Copy link
Contributor

anadon commented Nov 1, 2018

@wygos Upstream devel has TravisCI tests which your is missing. Your tests are also failing. Please rebase.

@wygos wygos force-pushed the develop branch 3 times, most recently from f531f8f to 216fc39 Compare November 16, 2018 20:34
@wygos wygos force-pushed the develop branch 2 times, most recently from f531f8f to 60b4fca Compare November 22, 2018 15:35
    When the distance map is given and the newly discovered distance
    is worse than the distance stored in the distance map,
    the vertex is not even added to the priority queue.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants