Re: [DISCUSS] Depth First Repeat step

2018-05-20 Thread Keith Lohnes
I had a little bit of time this weekend so I've updated the PR that I'd submitted https://github.com/apache/tinkerpop/pull/838 with something that seems to work, at least with the cursory testing I've done so far and jives better with the sentiments expressed in this thread. It still uses a

Re: [DISCUSS] Depth First Repeat step

2018-05-17 Thread Stephen Mallette
I got a bit busy for a few weeks and didn't' really have much time to focus on in on this thread. I figured I'd let it off my mind for a while and see if any new thoughts came up on it. Coming back on it now, I still think I like the approach I summarized from my own thoughts and others on the

Re: [DISCUSS] Depth First Repeat step

2018-05-01 Thread Keith Lohnes
I presume that would be some source of technical derailment to this approach Not necessarily, just the “step to define traversal order” idea. The RepeatStep, at least from what I’ve seen, seems simplest to implement as a BFS. BFS is easily made depth first with a stack. I think that can still be

Re: [DISCUSS] Depth First Repeat step

2018-04-28 Thread Stephen Mallette
> Is the objection to order(SearchAlgo) that it overloads order() or an objection to specifying, DFS/BFS in the traversal itself? for me, order() is a step. not a modulator. making it work as presented in the pull request conflates those two concepts which i think is confusing (i don't think we

Re: [DISCUSS] Depth First Repeat step

2018-04-28 Thread pieter gmail
Hi, Is the objection to order(SearchAlgo) that it overloads order() or an objection to specifying, DFS/BFS in the traversal itself? If so I do not really see how it is misplaced from a usability/API perspective. Seems pretty natural to me and very graphy at that. As mentioned earlier I am

Re: [DISCUSS] Depth First Repeat step

2018-04-27 Thread Stephen Mallette
It seems like we have general agreement on the easy things, that is: 1. this is a change for 3.4.0/master and 2. we're all for a DFS option but we still have the hard part of having to come to consensus on how it's used/implemented. The quick summary of this thread in that regard goes something

Re: [DISCUSS] Depth First Repeat step

2018-04-24 Thread Keith Lohnes
Yeah, that's what I meant. The steps inside are replaced with some JanusGraph stuff. Cheers, Keith On Tue, Apr 24, 2018 at 1:52 PM pieter gmail wrote: > Nah, that looks to me like the RepeatStep survived. Just the nested > VertexStep that got replaced with

Re: [DISCUSS] Depth First Repeat step

2018-04-24 Thread pieter gmail
Nah, that looks to me like the RepeatStep survived. Just the nested VertexStep that got replaced with JanusgraphVertexStep. Good for them, first prize is not replacing anything. Cheers Pieter On 24/04/2018 19:50, Keith Lohnes wrote: It looks like it, `g.V().has("foo",

Re: [DISCUSS] Depth First Repeat step

2018-04-24 Thread Keith Lohnes
It looks like it, `g.V().has("foo", "bar").repeat(out()).emit().explain()` yields `[JanusGraphStep([],[foo.eq(bar)]), RepeatStep([JanusGraphVertexStep(OUT,vertex), RepeatEndStep],until(false),emit(true))]` On Tue, Apr 24, 2018 at 12:12 PM pieter gmail wrote: > Hi, >

Re: [DISCUSS] Depth First Repeat step

2018-04-24 Thread pieter gmail
Hi, Sqlg completely replaces TinkerPop's RepeatStep. The idea being that with g.V().repeat(out()).times(x) only x round trips to the db is needed regardless of the size of the graph. Each time it will go to the db with the full set of the previous step's incoming starts. But yeah

Re: [DISCUSS] Depth First Repeat step

2018-04-24 Thread Keith Lohnes
Pieter, If you take a look at https://github.com/apache/tinkerpop/pull/838 DFS is implemented as a modification to BFS. It's taking the starts that come in from a BFS and stashing them to be processed later. I haven't seen a big performance difference on JanusGraph; At least for the queries that

Re: [DISCUSS] Depth First Repeat step

2018-04-19 Thread pieter gmail
Hi, Not really sure either what 'global' means technically with respect to TinkerPop's current configuration support. Perhaps it can be the start of a global configuration registry that can be overridden per traversal. I get that DFS is the preferred default but for Sqlg the performance

Re: [DISCUSS] Depth First Repeat step

2018-04-19 Thread Keith Lohnes
> whether this will affect more than just repeat()? For the PR that I start and the intent of this thread was to only affect repeat. > I prefer that the semantics of the traversal be specified in the traversal as a first class citizen. +1 > I am fine with any default but am wondering whether

Re: [DISCUSS] Depth First Repeat step

2018-04-17 Thread Daniel Kuppitz
TinkerPop makes no guarantees about the order of elements unless you specify an explicit order. This also goes back to the fact that certain strategies (LazyBarrier-, RepeatUnroll- and PathRetractionStrategy) add NoOpBarrierSteps to your traversal, which ultimately turns it into a DFS/BFS mix.

Re: [DISCUSS] Depth First Repeat step

2018-04-17 Thread Michael Pollmeier
> Also it seems to me that DFS only really applies to repeat() with an > emit(). > g.V().hasLabel("A").repeat().times(2) gets rewritten as > g.V().hasLabel("A").out().out(). Are their subtleties that I am not > aware of or does DFV vs BFS not matter in this case? When I read this I thought:

Re: [DISCUSS] Depth First Repeat step

2018-04-17 Thread pieter gmail
Hi, I agree with the question about whether this will affect more than just repeat()? I prefer that the semantics of the traversal be specified in the traversal as a first class citizen. i.e. with order(SearchAlgo). Strategies are to my mind internal to an implementation. In Robert's

Re: [DISCUSS] Depth First Repeat step

2018-04-17 Thread Robert Dale
+1 on DFS -1 on order(SearchAlgo) It seems like a strategy may be more appropriate. It could affect more than just repeat(). And how would this interact with LazyBarrierStrategy? Maybe the default should be DFS with LazyBarrierStrategy. Then LazyBarrierStrategy can be removed with

Re: [DISCUSS] Depth First Repeat step

2018-04-17 Thread Stephen Mallette
Thanks for the summary. Just a quick note - I'd not worry about the GLV tests for now. That part should be easy to sort out. Let's first make sure that we get clear on the other items first before digging too deeply there. On an administrative front, I think that this change should just go to

Re: [DISCUSS] Depth First Repeat step

2018-04-17 Thread Keith Lohnes
Stephen, That’s a fair summary. I had an immediate need for it, so I developed something based on Michel Pollmeier’s work and a modification to the syntax Pieter Martin suggested in the Jira

Re: [DISCUSS] Depth First Repeat step

2018-04-16 Thread Stephen Mallette
Keith, I have to admit that I didn't really follow this general body of work closely, which include this pull request, the earlier one from Michael Pollmeier, the JIRA and I think some discussion somewhere on one of the mailing lists. As best I have gathered from what I did follow, I feel like

Re: [DISCUSS] Depth First Repeat step

2018-04-16 Thread pieter gmail
Hi, I have not properly followed the previous thread. However I thought is going to be a way to set the default behavior as apposed to needing to use barrier() Is this the case or not? For Sqlg at least it is possible to optimize BFS much more effectively than DFS so it will be nice to have

Re: [DISCUSS] Depth First Repeat step

2018-04-16 Thread Daniel Kuppitz
+1 for DFS. If the query relies on BFS, you can always do .repeat(barrier())... ^ This holds true as long as there's no significant difference in the cpu+memory consumption and overall performance of the two approaches. BFS has its advantages when it comes to bulking; an arbitrary number of

[DISCUSS] Depth First Repeat step

2018-04-16 Thread Keith Lohnes
As part of #838 there’s some discussion around whether or not to make DFS the default for the repeat step. On the one hand, everything else in OLTP is depth first. On the other hand, there’s likely existing traversals that depend on the breadth first