Github user vasia commented on the pull request:
https://github.com/apache/flink/pull/537#issuecomment-102366422
Hey @andralungu! Thanks a lot for looking into this!
Can you please close this PR and I'll open a new one with the latest
changes.
Regarding returning -1 if
Github user andralungu closed the pull request at:
https://github.com/apache/flink/pull/537
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is
Github user andralungu commented on the pull request:
https://github.com/apache/flink/pull/537#issuecomment-101978637
Hi @vasia,
I had a look at the new branch. The changes look good, degrees are no
longer exposed to the user and the current approach removes the need to
Github user vasia commented on the pull request:
https://github.com/apache/flink/pull/537#issuecomment-101048928
Hey @andralungu ,
I built on your changes and implemented what I described above. I have
pushed in [this
Github user vasia commented on the pull request:
https://github.com/apache/flink/pull/537#issuecomment-99070508
Hey @andralungu ,
I'm not sure what @StephanEwen had in mind when suggesting to subclass
Vertex, but the current implementation forces the user to cast the `Vertex`
Github user andralungu commented on the pull request:
https://github.com/apache/flink/pull/537#issuecomment-99232613
Hey @vasia ,
Everyone is acting as though I do not want to make these methods
user-freindly. I do :), it's just not possible.
Keep in mind that in
Github user andralungu commented on the pull request:
https://github.com/apache/flink/pull/537#issuecomment-97159494
Thanks for the review, @StephanEwen!
PR updated.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well.
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/537#issuecomment-96558324
Looks good in general. A few thoughts on this:
Your prior discussion involved efficiency, and Vasia suggested to not carry
the degrees in all cases (as they
Github user andralungu commented on a diff in the pull request:
https://github.com/apache/flink/pull/537#discussion_r28811435
--- Diff:
flink-staging/flink-gelly/src/test/java/org/apache/flink/graph/test/example/IncrementalSSSPITCase.java
---
@@ -0,0 +1,84 @@
+/*
+ *
Github user andralungu commented on a diff in the pull request:
https://github.com/apache/flink/pull/537#discussion_r28764335
--- Diff:
flink-staging/flink-gelly/src/main/java/org/apache/flink/graph/spargel/VertexUpdateFunction.java
---
@@ -40,7 +42,37 @@
VertexValue
Github user andralungu commented on a diff in the pull request:
https://github.com/apache/flink/pull/537#discussion_r28785218
--- Diff:
flink-staging/flink-gelly/src/main/java/org/apache/flink/graph/spargel/VertexCentricIteration.java
---
@@ -138,69 +146,46 @@ public void
Github user andralungu commented on the pull request:
https://github.com/apache/flink/pull/537#issuecomment-94915061
Hi @vasia ,
I added some answers to your inline comments! I will push my latest version
for this tomorrow.
Regarding the suggestion that the Vertex
Github user vasia commented on a diff in the pull request:
https://github.com/apache/flink/pull/537#discussion_r28769333
--- Diff:
flink-staging/flink-gelly/src/main/java/org/apache/flink/graph/spargel/VertexUpdateFunction.java
---
@@ -40,7 +42,37 @@
VertexValue extends
Github user vasia commented on a diff in the pull request:
https://github.com/apache/flink/pull/537#discussion_r28643293
--- Diff: docs/gelly_guide.md ---
@@ -380,6 +380,16 @@ all aggregates globally once per superstep and makes
them available in the next
*
Github user vasia commented on a diff in the pull request:
https://github.com/apache/flink/pull/537#discussion_r28644140
--- Diff:
flink-staging/flink-gelly/src/main/java/org/apache/flink/graph/spargel/VertexUpdateFunction.java
---
@@ -40,7 +42,37 @@
VertexValue extends
Github user vasia commented on a diff in the pull request:
https://github.com/apache/flink/pull/537#discussion_r28643290
--- Diff: docs/gelly_guide.md ---
@@ -380,6 +380,16 @@ all aggregates globally once per superstep and makes
them available in the next
*
Github user vasia commented on a diff in the pull request:
https://github.com/apache/flink/pull/537#discussion_r28643470
--- Diff:
flink-staging/flink-gelly/src/main/java/org/apache/flink/graph/example/IncrementalSSSPExample.java
---
@@ -0,0 +1,304 @@
+/*
+ * Licensed to
Github user vasia commented on a diff in the pull request:
https://github.com/apache/flink/pull/537#discussion_r28643473
--- Diff:
flink-staging/flink-gelly/src/main/java/org/apache/flink/graph/example/IncrementalSSSPExample.java
---
@@ -0,0 +1,304 @@
+/*
+ * Licensed to
Github user vasia commented on a diff in the pull request:
https://github.com/apache/flink/pull/537#discussion_r28643463
--- Diff:
flink-staging/flink-gelly/src/main/java/org/apache/flink/graph/example/IncrementalSSSPExample.java
---
@@ -0,0 +1,304 @@
+/*
+ * Licensed to
Github user vasia commented on a diff in the pull request:
https://github.com/apache/flink/pull/537#discussion_r28643683
--- Diff:
flink-staging/flink-gelly/src/main/java/org/apache/flink/graph/spargel/VertexCentricIteration.java
---
@@ -24,18 +24,24 @@
import
Github user vasia commented on the pull request:
https://github.com/apache/flink/pull/537#issuecomment-94167112
Hi @andralungu! Thanks for the update :-)
I left a few inline comments. Overall it looks good, I just had a little
trouble following the logic in
Github user vasia commented on a diff in the pull request:
https://github.com/apache/flink/pull/537#discussion_r28643437
--- Diff:
flink-staging/flink-gelly/src/main/java/org/apache/flink/graph/example/IncrementalSSSPExample.java
---
@@ -0,0 +1,304 @@
+/*
+ * Licensed to
Github user vasia commented on a diff in the pull request:
https://github.com/apache/flink/pull/537#discussion_r28643520
--- Diff:
flink-staging/flink-gelly/src/main/java/org/apache/flink/graph/spargel/IterationConfiguration.java
---
@@ -189,4 +199,33 @@ public void
Github user vasia commented on a diff in the pull request:
https://github.com/apache/flink/pull/537#discussion_r28643695
--- Diff:
flink-staging/flink-gelly/src/main/java/org/apache/flink/graph/spargel/VertexCentricIteration.java
---
@@ -86,7 +92,9 @@
private
Github user vasia commented on a diff in the pull request:
https://github.com/apache/flink/pull/537#discussion_r28644150
--- Diff:
flink-staging/flink-gelly/src/main/java/org/apache/flink/graph/spargel/MessagingFunction.java
---
@@ -38,25 +42,57 @@
* @param Message The type
Github user vasia commented on a diff in the pull request:
https://github.com/apache/flink/pull/537#discussion_r28643344
--- Diff: docs/gelly_guide.md ---
@@ -380,6 +380,16 @@ all aggregates globally once per superstep and makes
them available in the next
*
Github user vasia commented on a diff in the pull request:
https://github.com/apache/flink/pull/537#discussion_r28644120
--- Diff:
flink-staging/flink-gelly/src/test/java/org/apache/flink/graph/test/example/IncrementalSSSPITCase.java
---
@@ -0,0 +1,84 @@
+/*
+ * Licensed
Github user vasia commented on a diff in the pull request:
https://github.com/apache/flink/pull/537#discussion_r28643494
--- Diff:
flink-staging/flink-gelly/src/main/java/org/apache/flink/graph/library/SingleSourceShortestPaths.java
---
@@ -77,7 +77,7 @@ public Double
Github user vasia commented on a diff in the pull request:
https://github.com/apache/flink/pull/537#discussion_r28643926
--- Diff:
flink-staging/flink-gelly/src/main/java/org/apache/flink/graph/spargel/VertexCentricIteration.java
---
@@ -138,69 +146,46 @@ public void
Github user vasia commented on a diff in the pull request:
https://github.com/apache/flink/pull/537#discussion_r28643920
--- Diff:
flink-staging/flink-gelly/src/main/java/org/apache/flink/graph/spargel/VertexCentricIteration.java
---
@@ -138,69 +146,46 @@ public void
Github user vasia commented on the pull request:
https://github.com/apache/flink/pull/537#issuecomment-92301058
Hey @andralungu!
Now that #547 is merged, the degrees, total number of vertices and
direction can be added as configuration options. This should also simplify
writing
Github user andralungu commented on the pull request:
https://github.com/apache/flink/pull/537#issuecomment-90728415
Hi @vasia ,
I simply assumed that if the previous examples work, the iteration also
works as before. Would you like a specific test suite for that?
Github user vasia commented on the pull request:
https://github.com/apache/flink/pull/537#issuecomment-90233701
Hey @andralungu! Thanks a lot for your work on this :)
I only took a quick look at your changes for now, but it seems that all
issues are fixed.
This PR
Github user andralungu commented on the pull request:
https://github.com/apache/flink/pull/537#issuecomment-90061271
@vasia ,
PR updated :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does
Github user andralungu commented on the pull request:
https://github.com/apache/flink/pull/537#issuecomment-87425953
Hi @vasia ,
1). I will add the documentation once we reach a consensus. No use
documenting something that might change :)
2). For the public setter: I
Github user vasia commented on the pull request:
https://github.com/apache/flink/pull/537#issuecomment-87462312
Hi @andralungu!
Regarding (2), I think you could do that inside VertexCentricIteration
instead of inside a method of Graph.
Regarding (3), I agree that adding
Github user andralungu commented on a diff in the pull request:
https://github.com/apache/flink/pull/537#discussion_r27344502
--- Diff:
flink-staging/flink-gelly/src/main/java/org/apache/flink/graph/spargel/MessagingFunction.java
---
@@ -38,11 +41,41 @@
* @param Message The
Github user vasia commented on the pull request:
https://github.com/apache/flink/pull/537#issuecomment-86727594
Hey @andralungu! Thanks for this PR. I know it took more effort than you
might have thought initially :-)
I have a few comments:
- In my opinion, the degrees
Github user vasia commented on a diff in the pull request:
https://github.com/apache/flink/pull/537#discussion_r27256989
--- Diff:
flink-staging/flink-gelly/src/main/java/org/apache/flink/graph/spargel/MessagingFunction.java
---
@@ -38,11 +41,41 @@
* @param Message The type
Github user vasia commented on a diff in the pull request:
https://github.com/apache/flink/pull/537#discussion_r27257390
--- Diff:
flink-staging/flink-gelly/src/main/java/org/apache/flink/graph/spargel/VertexUpdateFunction.java
---
@@ -40,7 +41,22 @@
VertexValue extends
Github user vasia commented on a diff in the pull request:
https://github.com/apache/flink/pull/537#discussion_r27255825
--- Diff:
flink-staging/flink-gelly/src/main/java/org/apache/flink/graph/Graph.java ---
@@ -1118,17 +1118,61 @@ public boolean filter(EdgeK, EV edge) {
Github user vasia commented on a diff in the pull request:
https://github.com/apache/flink/pull/537#discussion_r27256784
--- Diff:
flink-staging/flink-gelly/src/main/java/org/apache/flink/graph/library/LabelPropagation.java
---
@@ -68,12 +70,12 @@ public LabelPropagation(int
42 matches
Mail list logo