STL Priority queue with custom comparator not working as expected

Question

I'm trying to implement Priority queue with custom operator. The algorithm tries to find the minimum increments to be done so that no two adjacent elements in array have absolute difference > 1.
For this, I get the maximum element "x" in the array and modify its neighbours to x-1, then repeat the same for other elements
Here's the code:

#include<iostream>
#include<algorithm>
#include<vector>
#include<queue>
using namespace std;

int arr[100], visited[100];
int sizeOfArray;
struct comp
{
public:
    bool operator() (int x1, int x2) {
    return arr[x1] < arr[x2];
    }
};  

int main(){
    cin>>sizeOfArray;
    priority_queue<int, vector<int>, comp> priorityQue;
    for(int i = 0; i < sizeOfArray; i++) {
        cin>>arr[i];
        priorityQue.push(i);
        visited[i]=0;
    }
    while(!priorityQue.empty()) {
        int index = priorityQue.top();
        priorityQue.pop();
        if(visited[index])   
            continue;
        visited[index]=1;

        cout<<"Currently at index: "<<index<<endl;

        int currentElement = arr[index];
        int dx[] = {-1, 1};    // left and right neighbours
        for(int k = 0; k < 2; k++) {
            int nextIndex = index + dx[k];
            if( nextIndex >= 0 && nextIndex < sizeOfArray && 
                (currentElement - arr[nextIndex]) > 1 ) 
            {
                arr[nextIndex] = currentElement - 1;
                cout<<"Modifying index :"<<nextIndex<<endl;
                cout<<"New Array is: ";
                // print array 
                for(int z=0;z<sizeOfArray;z++)
                    cout<<arr[z]<<" ";
                cout<<endl;
                
                priorityQue.push(nextIndex);
                cout<<"Pushing index "<<nextIndex<<" to queue"<<endl;
            }
        }
    }
    return 0;
}

For the input:

4
4 1 1 0

The Output is:

Currently at index: 0
Modifying index :1
New Array is: 4 3 1 0
Pushing index 1 to queue
Currently at index: 2
Currently at index: 1
Modifying index :2
New Array is: 4 3 2 0
Pushing index 2 to queue
Currently at index: 3

I found that priority queue is not extracting the maximum element as per the comparator as it should be. After visiting index 0, array becomes 4 3 1 0 hence index 1 should come next but in this case index 2 is picked up. What am I missing??

Answer 1

You are modifying the item after it was put into the queue and still is part of that queue. This is not supported by the priority queue and yields unspecified results. C++ STL does not offer updatable priority queue.

But, seeing your use case I suspect this is competitive algorithmic programming. There are decent alternetives for this use case. I wouldn't recommend them for production code (at least not without proper abstractions.

The first, "proper" alternative is to use std::set<std::pair<...>> instead. Your code will stay very similar, but there are some important differences:

• You won't need custom comparator, instead relying on pair comparison (you will need to use std::greater as comparator to have largest items "on top",
• You will put {a[index], index} as elements of this set,
• You will use .begin() instead of .top(),
• You will need to .erase() items before updating their values, and insert them again with new values.

I believe above has same complexities as the standard implementation of priority queue. Even though updates seem slow, we are doing just two O(log n) operations - which is same complexity as an actual update in a heap structure.

You could implement it with an indirect comparator as in your example. In general it would work, but you still need the erase and insert flow around update. Additionally you would need to compare indices in your comparator as well, in order to make items with same priority different, so that the correct item is removed.

Moreover, there is another trick we can do in many common cases, like Dijkstra or Prim's algorithms. In these cases we only update priority to higher (lower values). In such case we can ignore erasing items, and instead just add duplicates. This works because time complexity of a single query/update becomes O(log n^2) = O(2 log n) = O(log n). Memory complexity increases, but this often is not a problem.

In this last case you can use other containers to fit your preferences, both std::priority_queue<std::pair<...>> or std::multimap<...> will work nicely. But in all of them you need to keep priority as part of item you insert, and not use it via indirect comparator.

As an appendix, this is your code with changes to work as intended:

#include<iostream>
#include<algorithm>
#include<vector>
#include<queue>
using namespace std;

int arr[100], visited[100];
int sizeOfArray;


int main(){
    cin>>sizeOfArray;
    priority_queue<std::pair<int, int>> priorityQue;
    for(int i = 0; i < sizeOfArray; i++) {
        cin>>arr[i];
        priorityQue.push({arr[i], i});
        visited[i]=0;
    }
    while(!priorityQue.empty()) {
        int index = priorityQue.top().second;
        priorityQue.pop();
        if(visited[index])   
            continue;
        visited[index]=1;

        cout<<"Currently at index: "<<index<<endl;

        int currentElement = arr[index];
        int dx[] = {-1, 1};    // left and right neighbours
        for(int k = 0; k < 2; k++) {
            int nextIndex = index + dx[k];
            if( nextIndex >= 0 && nextIndex < sizeOfArray && 
                (currentElement - arr[nextIndex]) > 1 ) 
            {
                arr[nextIndex] = currentElement - 1;
                cout<<"Modifying index :"<<nextIndex<<endl;
                cout<<"New Array is: ";
                // print array 
                for(int z=0;z<sizeOfArray;z++)
                    cout<<arr[z]<<" ";
                cout<<endl;
                
                priorityQue.push({arr[nextIndex], nextIndex});
                cout<<"Pushing index "<<nextIndex<<" to queue"<<endl;
            }
        }
    }
    return 0;
}