Skip to content

Add self loop example to Cycle Detection recipe.#921

Merged
asfgit merged 1 commit into
apache:tp32from
newkek:cycles-recipe-self-loops
Aug 31, 2018
Merged

Add self loop example to Cycle Detection recipe.#921
asfgit merged 1 commit into
apache:tp32from
newkek:cycles-recipe-self-loops

Conversation

@newkek
Copy link
Copy Markdown
Contributor

@newkek newkek commented Aug 24, 2018

Not sure if it is worth it but I figured out that self loops weren't detected in the given example.

addE('knows').from('c').to('d').
addE('self').from('a').to('a').iterate()
g.V().as('a').emit().repeat(out().simplePath()).times(2).
where(out().as('a')).path()
Copy link
Copy Markdown
Contributor

@dkuppitz dkuppitz Aug 25, 2018

Choose a reason for hiding this comment

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

I would tweak the traversal a little bit to make the output easier to grasp:

gremlin> g.V().as('a').
......1>   emit().
......2>     repeat(outE().inV().simplePath()).
......3>     times(2).
......4>   outE().inV().where(eq('a')).
......5>   path().
......6>     by(id).
......7>     by(label)
==>[a,self,a]
==>[a,knows,b,knows,c,knows,a]
==>[b,knows,c,knows,a,knows,b]
==>[c,knows,a,knows,b,knows,c]

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good point!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@dkuppitz what is the added value of outE().inV() compared to out()?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Really just readability. To me [a,self,a] looks better than just [a,a].

by(id).
by(label)
----

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cool, much better. What I didn't notice in the first place, was that the paragraph below really only refers to the previous code snippets. I think you should move it above you changes.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hm the issue with the Traversal below is that it improves things further by formatting results more and doing unidirectional traversing on an unrestricted path length. Since the examples get more complicated as you progress through the document, if I put this self loop traversal under the more complicated one it feels like going backwards. Additionally the traversal below refers to the modern graph, whereas the first one and this self-loop one are similar, wdyt?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I mean just put the paragraph that starts with

The above case assumed that ...

above your new block that starts with

Note that these ...

That would make more sense, no?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The above case assumed that the need was to only detect cycles over a path length of three.
It also respected the directionality of the edges by only considering outgoing ones.

Also note that this traversal won't detect self-loops [....]
--
[self-loop code example]
--

What would need to change to detect cycles of arbitrary length over both incoming and
outgoing edges, in the modern graph?

--
[bi-directional code example]
--

[...]

Is that what you meant?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yep.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Okay. well no strong opinion here I can change it for that

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

@dkuppitz
Copy link
Copy Markdown
Contributor

VOTE +1

@newkek newkek force-pushed the cycles-recipe-self-loops branch from 5dfcdfc to 3afc576 Compare August 31, 2018 19:36
@newkek newkek changed the base branch from master to tp32 August 31, 2018 19:48
@asfgit asfgit merged commit 3afc576 into apache:tp32 Aug 31, 2018
@newkek newkek deleted the cycles-recipe-self-loops branch September 5, 2018 14:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants