Solved priority queue design!

[TimLovellSmith]

This is intended to address issue #1841 and all other design problems for priority queue. Now can .net finally have a priority queue please? :)

[ahsonkhan]

cc @safern, @ianhays, @KrzysztofCwalina
Link to original discussion (and specific request for this PR): #574.

[ahsonkhan]

Similar to this change (changing heading from Scenario to Examples and updating the numbers), consider changing it here too: https://github.com/TimLovellSmith/corefxlab/blob/d61eda595a030aee4f0563bebb400052d8ab0c13/docs/specs/priority-queue.md#scenario

[ianhays]

I like it.

[nietras]

I can concur that we really need a PriorityQueue, but preferable one that allows updates of the priority based on the element. E.g. add a method (which does not seem to be part of proposal):

public class PriorityQueue<TElement, TPriority>
{
    public void Update(TElement element, TPriority priority);
}

And preferable with O(1) complexity. This allows this to be used for say Dijkstra's shortest path for example.

Also consider that it might be beneficial to also be able to query in constant time also the "tail" of the priority queue. SortedSet<T> has Min/Max for that. Here it seems Peek if for getting the "head".

[TimLovellSmith]

Hi @nietras
I think you didn't understand that the design's current intention is to allow updates of the priority based on the element, in the 'external priority' case - it is done simply like this:

priorityQueue[elementKey] = newValue;

And yes, this is definitely intended to have O(1) amortized complexity and be usable for Dijkstra's algorithm. (Oops, not so sure about O(1) now, more in below discussion.)

The suggestion to have constant time query of back of the priority queue is an interesting one. The main drawback I foresee is that it is not easy to implement efficiently unless your priority queue is in fact a sorted set underneath.

[nietras]

@TimLovellSmith ah I missed that. My mistake. Great that sounds good.

it is not easy to implement efficiently unless your priority queue is in fact a sorted set underneath.

No you are right, SortedSet can be used for that but it doesn't handle some of the use cases of priority queue. Min-max can be handled by two priority queues too, so better to be really good at the normal use case which is either min or max-heap.

Will there be an experimental implementation of this soon?

[TimLovellSmith]

Here's a few updates + fixes based on the discussion so far, thanks for feedback! I think it might be clear enough for someone to have a go at implementing now.
@nietras I'm now not sure the interface itself would really guarantee O(1) update key, I think it might only guarantee something a little easier to achieve like O(log(n)). It is intended to support increase key as well as decrease key for completeness.

[nietras]

not sure the interface itself would really guarantee O(1) update key,

@TimLovellSmith that was my concern. Should it not be considered then to add a say IncreasePriority(TKey key, TPriority priority) or something named like that to the interface to ensure relaxation can be done optimally?

[TimLovellSmith]

@nietras Well, I also have some doubts about whether an implementation would actually be the fastest for typical everyday use cases even though its O(1). But I figure that kind of detail can perhaps best be decided by implementors over time as opposed to trying to bake it into the interface.
Unless you are asking for something which is really a different operation from update-priority, like a conditional-update-priority?

[nietras]

figure that kind of detail can perhaps best be decided by implementors over time as opposed to trying to bake it into the interface

@TimLovellSmith I agree 😄

Unless you are asking for something which is really a different operation from update-priority, like a conditional-update-priority?

No basically, just "increase-key" (or "decrease-key" depending on comparer), which I assume can be detected when the index operator is used.

One thing, I am not absolutely happy about is the fact that one cannot use a "value type" comparer, to avoid potential virtual call costs on compares. But no other type in .NET does that so that is probably too involved.

[nietras]

ISet<T> does not seem entirely appropriate. Maybe.

[ianhays]

@TimLovellSmith this has been open for a while, are you happy with the current state or do you have more changes to make?

[TimLovellSmith]

@ianhays @nietras
Thanks for all the feedback. My thinking now about PrioritySet has since become that it was a cute idea but not a useful and good idea. Its not useful because you can just do every PrioritySet scenario using PriorityQueue instead. And its not good because it is a pit of failure with inconsistent data structure issues, I'm sure that's not the kind of thing standard library maintainers will want to help people debug.

I still think it is nice for PriorityQueue to be a dictionary, since it should be a 1-1 mapping - every queue item should have a unique priority.

I'm going to do one more revision to remove PrioritySet from the design.

[nietras]

A quick note, I really don't want a 1-1 mapping allowing only unique keys, there's SortedDictionary for that. This proposal loses most it's value then.

…

On Jan 18, 2018 19:04, "Tim Lovell-Smith" ***@***.***> wrote: @ianhays <https://github.com/ianhays> @nietras <https://github.com/nietras> Thanks for all the feedback. My thinking now about PrioritySet has since become that it was a cute idea but not a useful and good idea. Its not useful because you can just do every PrioritySet scenario using PriorityQueue instead. And its not good because it is a pit of failure with inconsistent data structure issues, I'm sure that's not the kind of thing standard library maintainers will want to help people debug. I still think it is nice for PriorityQueue to be a dictionary, since it should be a 1-1 mapping - every queue item should have a unique priority. I'm going to do one more revision to remove PrioritySet from the design. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1850 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AKTG79bjg97tHqSNYSKFuCzF5Q4RY6yaks5tL4emgaJpZM4P7YyK> .

[TimLovellSmith]

@nietras What scenario would you need to give the same item multiple priorities for? And would you use update priority in that scenario?
IMO PriorityQueue's main value is its different algorithmic complexity guarantees compared to sorted dictionary.

[TimLovellSmith]

@nietras
I should probably write some more about my thinking on why no duplicates. Then I might realize whether it is wrong or not...

I think the hole the previous proposals have been trying to fill is that there's no good data structure in the framework for 'set of unique objects in priority order'. The objects could be either reference or value types as long as they are unique... the desire in this scenario is of course to be able to do efficient dequeue-min and change-priority operations.

But it is also ideal to have a priority queue that supports value types with duplicates allowed.
e.g. you can just do a priority queue of numbers as well as objects.
This tends to be a pattern of 'the object is the priority'.
This scenario tends to not require any priority key updates, it instead works by removal / adding to the queue. Pretty similar to SortedList you would think from hearing the name - except it is not, because SortedList is really a dictionary of unique keys! It is almost the same as the PriorityDictionary proposed here except that it hasn't got the desired performance characteristics

And actually until now this proposal is doing a pretty poor job of supporting the scenario of distrinct keys, ...

I am updating the proposal to address that...

[danmoseley]

Note to self

https://github.com/Microsoft/Trill/blob/master/Sources/Core/Microsoft.StreamProcessing/Collections/PriorityQueue.cs

[ahsonkhan]

What's our plan for this PR?

[pgolebiowski]

Issue: Add a pairing heap to System.Collections.Specialized instead of this one.

[feliwir]

What's missing to get the PriorityQueue merged?