-
Notifications
You must be signed in to change notification settings - Fork 2
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
minmax dist heap #341
minmax dist heap #341
Conversation
return nil, EmptyHeapError | ||
} | ||
|
||
return Pop(d).(*Item), nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be fine to make the underlying heap coupled to Item
so we don't have to type cast. Generally, these type casts are pretty slow and we're the only user of the heap so we're paying for an abstraction that isn't necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gotcha -- however, I thought in order to implement the sort
Interface, I would have to abide by the function header and the any
param:
// The Interface type describes the requirements
// for a type using the routines in this package.
// Any type that implements it may be used as a
// min-heap with the following invariants (established after
// [Init] has been called or if the data is empty or sorted):
//
// !h.Less(j, i) for 0 <= i < h.Len() and 2*i+1 <= j <= 2*i+2 and j < h.Len()
//
// Note that [Push] and [Pop] in this interface are for package heap's
// implementation to call. To add and remove things from the heap,
// use [heap.Push] and [heap.Pop].
type Interface interface {
sort.Interface
Push(x any) // add x as element Len()
Pop() any // remove and return element Len() - 1.
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, I think actually we don't need to implement that interface. Since you've implemented the min/max variants it should be concretely-typed. In fact, the generic heap.Pop()
for example is underspecified since it doesn't inform whether to pop the min or the max.
It's not a great solution if this were a generic library that anyone can use but we're going to be the only consumers of this heap so it's fine if there's no interface it conforms to.
Closing for #342 |
The refactor was getting gnarly, this is the new priority queue used for all logic throughout hnsw