stingergraph / StingerGraphs.jl

Julialang bindings to the STINGER graph database
http://www.stingergraph.com
Other
5 stars 3 forks source link

Custom Array Type for Vertices? #8

Open rohitvarkey opened 7 years ago

rohitvarkey commented 7 years ago

While working on #4, I found that returning an Array{Int64} which is indexed by 1 might confuse users used to the zero based indexing in Stinger.

Maybe we can wrap these returned arrays using the shiny new custom abstract array types with non-1 indexing feature in 0.5. This will let users use zero based indexing on these arrays too!

jpfairbanks commented 7 years ago

I see the value in either convention. If you choose 0 based indexing, it will be more familiar to stinger users but if you choose 1 based indexing then it will be more familiar to Julia users.

Who do you want to cater to more?

rohitvarkey commented 7 years ago

The reason why I feel 0-based indexing is that it feels more intuitive to work with when it is an array of 0 based indices. For example, in the case of the bfs implementation.

s = Stinger()
insert_edge!(s, 0, 1, 5, 0, 0)
insert_edge!(s, 0, 5, 2, 0, 0)
insert_edge!(s, 0, 2, 0, 0, 0)
bfstree = bfs(s,1) #[2, -1, 5, -1, -1, 1]
bfstree[5] #This actually gives the parent for 4, which will be a -1.
bfstree[5+1] #This gives the correct parent for 5, which will be 1.

For Julia users using StingerWrapper, they are already using a 0-based indexing system to refer to the vertex IDs for insert_edge, remove_edge, etc. As you can see, off by one errors can come up quite easily with both these indexing systems in play.

jpfairbanks commented 7 years ago

Yeah I think that 0 based indexing is the right thing to do but it can wait until after we have the bfs benchmark numbers.