0

If std::vector<vector<pair<int,int> > > v(n) represents the adjacency list of the graph with pair<int,int> is the {vertex,weight} pair, I tried to implement the algorithm the following way: 

while (true)
{
    long long yo = LLONG_MAX;
    int ind = -1;
    for (int i = 0; i < n; ++i)
    {
        if (ans[i] < yo && !v[i].empty())
        {
            ind = i;
            yo = ans[i];
        }
    }
    if (ind == -1)
        break;
    for (int i = 0; i < v[ind].size(); ++i)
    {
        if (ans[v[ind][i].first] > ans[ind] + v[ind][i].second)
            ans[v[ind][i].first] = ans[ind] + v[ind][i].second;
        v[ind].erase(v[ind].begin() + i);
    }
}

Where ans[i] stores the shortest paths which is initialised as {LLONG_MAX,...0,...LLONG_MAX}, 0 being the source. Since this is the first time I tried implementing it, is there a better way to implement using the vectors/list in stl (in terms of time/space complexity maybe)?

Jarod42
  • 203,559
  • 14
  • 181
  • 302
yobro97
  • 1,125
  • 8
  • 23
  • 2
    This question might be better-suited for [Code Review](http://codereview.stackexchange.com), since it appears to be about code that already works. – cdhowie Apr 11 '17 at 16:44
  • "better" in terms of what? speed? memory usage? I personally value most readability ;) – 463035818_is_not_an_ai Apr 11 '17 at 16:45
  • @tobi303 Time and memory usage – yobro97 Apr 11 '17 at 16:46
  • @yobro97 If you want to optimize time and memory usage using a vector of vectors is already failure. – Nir Friedman Apr 11 '17 at 16:53
  • 2
    I would say that `v[ind].erase(v[ind].begin() + i);` is wrong. (as you modify vector during its iteration in a strange way). – Jarod42 Apr 11 '17 at 17:01
  • @NirFriedman *citation needed*. – n. m. could be an AI Apr 11 '17 at 20:02
  • @n.m. A vector of vectors only has any merit if the adjacency list can change. If the adjacency list is fixed then a vector of pairs, plus an auxiliary vector of offsets is strictly more efficient in space (at least for N larger than 3 or 4). And it is contiguous in memory so it will reduce cache missing as well. The difference may not be enormous (can only be resolved with benchmarks), but it will perform better. Think this is pretty clear to anyone experienced in writing performant C++. If I felt like doing the benchmark I would have written an answer not a comment. – Nir Friedman Apr 11 '17 at 20:27
  • Well apparently what is posted is not Dijkstra at all, so it makes little sense to discuss it, but in a real Dijkstra implementation the priority queue operations would probably dominate. – n. m. could be an AI Apr 11 '17 at 20:51
  • @Jarod42 Yes...I too had a confusion while erasing the vector elements. How shall I do then if I wish to proceed this way? – yobro97 Apr 12 '17 at 04:25

2 Answers2

0

Current approach has logical bug ( modifying the vector while iterating it ). Also Complexity wise it seems very bad as well, due to redundant while loop to get the new minimum distance node each time and un-necessary erase operation, which is heavy in vector due to shifting of the entire adjacency list.

I would recommend using a priority queue < min heap >, which is much cleaner and have a complexity of Elog(N), where E is no. of edges and N no. of node in graph. I am just copying pasting one of my old codes with comments.

#define P first
#define Q second 
typedef long long LL ;
typedef pair < LL , int > li ;
typedef pair < int , LL > il ;
dist[N] = {INT_MAX} ;  // array containing the distance to each node from source node 
vector < il > adj [100010] ; // adjacency list with (node, distance) pair 

void dijkstra( int s ){   // s is the source node
  priority_queue < li , vector < li > , greater < li > > pq; // priortiy queue with a pair <long, int> as the data , using vector as underlying data structure and std:greater comparator 
  dist [s] = 0;
  pq.push( li( dist[s], s) );
  while( !pq.empty() ){
      li u = pq.top() ; pq.pop() ; // u.P =  current minimum distance of node , u.Q = current node on top of the heap 
      if( dist[u.Q] == u.P )
      for( int i = 0 ; i  < adj[u.Q].size() ; i++){
           il v = adj[u.Q][i] ;
           if( dist[v.P] > u.P + v.Q ){
               dist[v.P] = u.P + v.Q ;
               pq.push( li(dis[v.P] ,v.P ));
           }
      } 
  }

If you have any questions, feel free to ask in comments.

Amit Kumar
  • 1,477
  • 1
  • 16
  • 27
0

Some ways to optimize Dijkstra:

  1. Use heap. Make sure you store vertices, not edges in the heap.
  2. Try bidirectional approach. Suppose, you have to find a shortest path from S to T. You can run 2 Dijkstra's simultaneously: one from S as usual, another one from T on reversed edges. You can switch between this 2 Dijkstras using some kind of heuristic. For example, work with Dijkstra with the smallest current radius.

Theoretically, Dijkstra can be optimized to O(E + VlogV) with Fibonacci heap. But in practice it works slower.

SashaMN
  • 708
  • 5
  • 16