Add a DataStructures module with PriorityQueue and heap functions. #2920

Merged
merged 6 commits into from Apr 30, 2013

Conversation

Projects
None yet
8 participants
Member

dcjones commented Apr 23, 2013

This is good to go, and everyone seems cool with it. See #2438.

Owner

ViralBShah commented Apr 23, 2013

I believe the module name Datastructures will conflict with the existing Datastructures.jl package.

Cc: @lindahua

Owner

ViralBShah commented Apr 23, 2013

An alternative name could be Collections.

Owner

ViralBShah commented Apr 23, 2013

It would be great if docs can be added before merging this.

Member

dcjones commented Apr 24, 2013

Now that you mention it, I think I prefer Collections to DataStructures. One word is better than two.

I've renamed and documented everything now.

Owner

ViralBShah commented Apr 24, 2013

exports.jl still seems to be exporting DataStructures.

Member

dcjones commented Apr 24, 2013

Oops. FIxed now.

@ViralBShah ViralBShah and 1 other commented on an outdated diff Apr 24, 2013

base/collections.jl
+ x[1]
+end
+
+
+# Unordered iteration through key value pairs in a PriorityQueue
+start(pq::PriorityQueue) = start(pq.index)
+
+done(pq::PriorityQueue, i) = done(pq.index, i)
+
+function next(pq::PriorityQueue, i)
+ (k, idx), i = next(pq.index, i)
+ return ((k, pq.xs[idx][2]), i)
+end
+
+
+end # module DataStructures
@ViralBShah

ViralBShah Apr 24, 2013

Owner

Silly - but I guess this should be Collections too.

@dcjones

dcjones Apr 24, 2013

Member

Certainly. Fixed now.

Owner

ViralBShah commented Apr 24, 2013

Thanks. I am going to leave this open for any other API discussions for a little bit.

@ViralBShah ViralBShah added a commit that referenced this pull request Apr 24, 2013

@ViralBShah ViralBShah Delete examples/{graph.jl,heap.jl} (#2598)
Heaps are added in #2920
Graphs.jl has sophisticated graph functionality.
Remove examples/RMT now that we have the RandomMatrices.jl package
ec1b200
Owner

stevengj commented Apr 24, 2013

Why not just have this in Base (as opposed to Base.Collections or whatever submodule)?

Member

dcjones commented Apr 25, 2013

There's a larger question of how Base should be organized, but with the addition of the Test and Sort modules we seem to be headed in the direction of not putting everything at top-level, so I'm following that trend.

Also, I expect there to be more in the Collections module eventually.

Member

lindahua commented Apr 27, 2013

At line 126 of collections.jl, index should be declared as Dict{K, Int} instead of Dict. I have experienced this many times: if the field types are not completely specific, you will see huge performance penalty. I sometimes see nearly 100x slow down if such a field is at the critical path.

I think a careful benchmark is needed to ensure that the implementation is optimized.

Owner

ViralBShah commented Apr 28, 2013

BTW, API-wise this looks good. I think it should be merged, but would like @StefanKarpinski and @JeffBezanson to look at this as well.

@lindahua Agree that we should benchmark this. That can be done even after this pull request is merged though. It would be great if Base.Test can have some support for performance benchmarking, coming to think of it. More generally, we need a way to track performance regressions.

Owner

JeffBezanson commented Apr 29, 2013

Please rebase, then I will merge it.

Member

dcjones commented Apr 30, 2013

Rebased!

Member

pao commented Apr 30, 2013

Priority queue tests do not pass: https://travis-ci.org/JuliaLang/julia/jobs/6747796#L602

Appears to be related to the changes in #2347.

Member

dcjones commented Apr 30, 2013

Thanks. It should be working now.

ViralBShah merged commit bbc1557 into master Apr 30, 2013

1 check passed

default The Travis build passed
Details

ViralBShah referenced this pull request May 7, 2013

Closed

0.2 release notes #2581

StefanKarpinski deleted the dcj/pq2 branch Nov 16, 2013

what should be value of Ordering... I mean how Ordering pass as argument... Can you show with an example like if one wants to create max priority queue what should be value of Ordering which must be passed

Contributor

ivarne replied Jul 3, 2014

abstract Ordering is just a trick we use because anonymous functions are currently slow in Julia. That means that instead of passing a function to the sorting method, we pass an object that has a specific implementation of the lt(obj::Ordering, a, b) function.

See how we define subtypes of Ordering and lt methods in: https://github.com/JuliaLang/julia/blob/e7694b6281f822f3dab4fddf72bff2da857ae62a/base/ordering.jl#L44

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment