Skip to content
This repository has been archived by the owner on Oct 21, 2021. It is now read-only.

Remove vertex_index(::Integer), make special case for SimpleGraph #135

Merged
merged 2 commits into from
Dec 21, 2014
Merged

Remove vertex_index(::Integer), make special case for SimpleGraph #135

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6c0b52f
Remove vertex_index(::Integer), make special case for SimpleGraph
mlubin Dec 21, 2014
b18c4bb
special case for ProvidedVertexType
mlubin Dec 21, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 2 additions & 9 deletions doc/source/vertex_edge.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ Vertices and Edges
Vertex Types
-------------

A vertex can be of any Julia type. For example, it can be an integer, a character, or a string. In a simplistic setting where there is no additional information associated with a vertex, it is recommended to use ``Int`` as the vertex type, which would lead to the best performance.
A vertex can be of any Julia type. For example, it can be an integer, a character, or a string.

This package provides two specific vertex types: ``KeyVertex`` and ``ExVertex``. The definition of ``KeyVertex`` is:

Expand Down Expand Up @@ -40,14 +40,7 @@ The ``ExVertex`` type implements a ``vertex_index`` function, as

returns the index of the vertex ``v``.

In addition, for integers, we have

.. code-block:: python

vertex_index(v::Integer) = v

This makes it convenient to use integers as vertices in graphs.

``SimpleGraph`` is a special case where the vertices are of type ``Int`` and store both their index and identity. In all other graphs, ``Int`` vertices are unordered indices.

Edge Types
-----------
Expand Down
1 change: 0 additions & 1 deletion src/adjacency_list.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ is_directed(g::GenericAdjacencyList) = g.is_directed

num_vertices(g::GenericAdjacencyList) = length(g.vertices)
vertices(g::GenericAdjacencyList) = g.vertices
vertex_index{V<:ProvidedVertexType}(v::V, g::GenericAdjacencyList{V}) = vertex_index(v)

num_edges(g::GenericAdjacencyList) = g.nedges

Expand Down
7 changes: 4 additions & 3 deletions src/common.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ typealias AttributeDict Dict{UTF8String, Any}
#
################################################

vertex_index(v::Integer) = v

immutable KeyVertex{K}
index::Int
key::K
Expand Down Expand Up @@ -37,7 +35,10 @@ typealias ProvidedVertexType Union(Integer, KeyVertex, ExVertex)

function vertex_index{V}(v::V, g::AbstractGraph{V})
@graph_requires g vertex_list
return vertex_index(v, vertices(g))
if applicable(vertex_index, v)
return vertex_index(v)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since applicable makes for more complex generated code and there's only one case where it's true, perhaps just writing out two separate methods instead? (I think it's clearer and no increase in lines of code, too.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would make it impossible to extend KeyVertex. Is the code generation actually bad for this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Double checked that it did not specialize how you'd hope (let me know if I did something wrong):

immutable A
    int::Int
end
immutable B
    int::Int
end

g(x) = x*x
f(::A, x::Int) = 2x
f(b::B) = b.int

a_only(a::A, x::Int) = f(a, g(x))
b_only(b::B, x::Int) = f(b)
function a_or_b{T}(v::T, x::Int)
    if applicable(f, v)
        return f(v)
    end
    return f(v, g(x))  
end

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know what constitutes bad -- it (and method_exists) just doesn't specialize like I expected the first time I used it:
Using that sample code above:

code_native(a_or_b2, (A, Int))
    .section    __TEXT,__text,regular,pure_instructions
Filename: none
Source line: 2
    push    RBP
    mov RBP, RSP
    push    R15
    push    R14
    push    R12
    push    RBX
    sub RSP, 32
    mov RBX, RSI
    mov R14, RDI
    mov QWORD PTR [RBP - 64], 4
Source line: 2
    movabs  R12, jl_pgcstack
    mov RAX, QWORD PTR [R12]
    mov QWORD PTR [RBP - 56], RAX
    lea RAX, QWORD PTR [RBP - 64]
    mov QWORD PTR [R12], RAX
    vxorps  XMM0, XMM0, XMM0
    vmovups XMMWORD PTR [RBP - 48], XMM0
    movabs  RAX, 140405752947744
Source line: 2
    mov RDI, QWORD PTR [RAX]
    mov RAX, QWORD PTR [RDI + 8]
    movabs  RCX, 140405770133600
    mov RCX, QWORD PTR [RCX]
Source line: 2
    lea R15, QWORD PTR [RBP - 48]
    movabs  RDX, 140405903671888
Source line: 2
    mov QWORD PTR [RBP - 48], RCX
    mov QWORD PTR [RBP - 40], RDX
    mov RSI, R15
    mov EDX, 2
    call    RAX
    movabs  RCX, 140405753013296
    cmp RAX, RCX
    je  L218
Source line: 3
    movabs  RAX, allocobj
    mov EDI, 16
    call    RAX
    movabs  RCX, 140405864207744
    mov QWORD PTR [RAX], RCX
    mov QWORD PTR [RAX + 8], R14
    mov QWORD PTR [RBP - 48], RAX
    movabs  RAX, jl_apply_generic
    movabs  RDI, 140405793447904
    mov RSI, R15
    mov EDX, 1
    call    RAX
    jmpq    L240
Source line: 5
L218:   imul    RBX, RBX
    add RBX, RBX
    movabs  RAX, jl_box_int64
    mov RDI, RBX
    call    RAX
L240:   mov RCX, QWORD PTR [RBP - 56]
    mov QWORD PTR [R12], RCX
    add RSP, 32
    pop RBX
    pop R12
    pop R14
    pop R15
    pop RBP
    ret

julia> code_native(a_only, (A, Int))
    .section    __TEXT,__text,regular,pure_instructions
Filename: none
Source line: 1
    push    RBP
    mov RBP, RSP
Source line: 1
    imul    RSI, RSI
    lea RAX, QWORD PTR [RSI + RSI]
    pop RBP
    ret

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll add a specialization for KeyVertex and leave this as it is for extensibility. The slow case where we have to do a linear search on indices is already slow and shouldn't be affected by this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, makes sense to me.

end
return vertex_index(v, vertices(g)) # slow linear search
end

vertex_index(v, vs::AbstractArray) = findfirst(vs, v)
Expand Down
1 change: 0 additions & 1 deletion src/edge_list.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ is_directed(g::GenericEdgeList) = g.is_directed

num_vertices(g::GenericEdgeList) = length(g.vertices)
vertices(g::GenericEdgeList) = g.vertices
vertex_index{V<:ProvidedVertexType}(v::V, g::GenericEdgeList{V}) = vertex_index(v)

num_edges(g::GenericEdgeList) = length(g.edges)
edges(g::GenericEdgeList) = g.edges
Expand Down
14 changes: 13 additions & 1 deletion src/graph.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@ vertices(g::GenericGraph) = g.vertices
num_edges(g::GenericGraph) = length(g.edges)
edges(g::GenericGraph) = g.edges

vertex_index{V<:ProvidedVertexType}(v::V, g::GenericGraph{V}) = vertex_index(v)
vertex_index(v::Integer, g::SimpleGraph) = (v <= g.vertices[end]? v: 0)

edge_index{V,E}(e::E, g::GenericGraph{V,E}) = edge_index(e)

out_edges{V}(v::V, g::GenericGraph{V}) = g.finclist[vertex_index(v, g)]
Expand All @@ -69,6 +70,17 @@ in_neighbors{V}(v::V, g::GenericGraph{V}) = SourceIterator(g, in_edges(v, g))

# mutation

function add_vertex!(g::SimpleGraph)
# ensure SimpleGraph indices are consecutive, allowing O(1) indexing
v = g.vertices[end] + 1
g.vertices = 1:v
push!(g.finclist, Int[])
push!(g.binclist, Int[])
v
end

@deprecate add_vertex!(g::SimpleGraph,v) add_vertex!(g::SimpleGraph)

function add_vertex!{V}(g::GenericGraph{V}, v::V)
push!(g.vertices, v)
push!(g.finclist, Int[])
Expand Down
1 change: 0 additions & 1 deletion src/incidence_list.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ vertices(g::GenericIncidenceList) = g.vertices

num_edges(g::GenericIncidenceList) = g.nedges

vertex_index{V<:ProvidedVertexType}(v::V, g::GenericIncidenceList{V}) = vertex_index(v)
edge_index{V,E}(e::E, g::GenericIncidenceList{V,E}) = edge_index(e)

out_edges{V}(v::V, g::GenericIncidenceList{V}) = g.inclist[vertex_index(v, g)]
Expand Down
3 changes: 2 additions & 1 deletion test/graph.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ using Base.Test
#
#################################################

sgd = simple_graph(4)
sgd = simple_graph(3)
add_vertex!(sgd)

# concept test

Expand Down