Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FLINK-1515]Splitted runVertexCentricIteration into createVertexCentricIteration and runVertexCentricIteration #402

Closed
wants to merge 2 commits into from

Conversation

martinkiefer
Copy link
Contributor

I changed the process for running a vertex-centric iteration in Gelly into a two-step procedure:

  1. createVertexCentricIteration returns a VertexCentricIteration object to allow developers to access features like broadcast data sets and aggregators.
  2. runVertexCentricIteration expects a VertexCentricIteration and returns the Graph.

This allows developers to use features like aggregators and broadcast data sets.

@vasia
Copy link
Contributor

vasia commented Feb 16, 2015

Hi @martinkiefer! Thanks a lot for this pull request!
The change looks good :)

I was only thinking that it would be nice to have an example that demonstrates how to use broadcast sets and / or aggregators now. It could simply be a slight modification of an existing example, e.g. SSSP with a counter of how many nodes have distance less than some value or similar. Could you make this addition?

@martinkiefer
Copy link
Contributor Author

I could change PageRank to read the number of vertices by broadcasting the result of numberofVertices() instead of providing it in the constructor of the algorithm. I guess, this is the use case that comes most naturally.

@vasia
Copy link
Contributor

vasia commented Feb 19, 2015

Thanks a lot @martinkiefer! +1 to merge from me :)

@vasia
Copy link
Contributor

vasia commented Feb 19, 2015

Oh, one minor thing.. Could you prefix your commits with "[FLINK-1515] [gelly]" ? Thanks!

…rtexIteration and runVertexIteration to make VertexCentricIteration object accessible to developers.
…y numberOfVertices() to read the number of vertices instead of passing it to the constructor.
@martinkiefer
Copy link
Contributor Author

Done

@cebe
Copy link
Contributor

cebe commented Feb 19, 2015

👍

@vasia
Copy link
Contributor

vasia commented Feb 20, 2015

Hi,
if no more comments, I'd like to merge this.
There is a failing check in Travis (not related to this PR):

Tests in error: 
  JobManagerFailsITCase.run:40->org$scalatest$BeforeAndAfterAll$$super$run:40->org$scalatest$WordSpecLike$$super$run:40->runTests:40->runTest:40->withFixture:40 » Timeout
  JobManagerFailsITCase.run:40->org$scalatest$BeforeAndAfterAll$$super$run:40->org$scalatest$WordSpecLike$$super$run:40->runTests:40->runTest:40->withFixture:40->TestKit.within:707->TestKit.within:707 » Timeout

Is this fixed by #422? Shall I proceed?
Thanks!

@tillrohrmann
Copy link
Contributor

I'm currently working on fixing this problem. You can ignore it for the
moment.

On Fri, Feb 20, 2015 at 11:58 AM, Vasia Kalavri notifications@github.com
wrote:

Hi,
if no more comments, I'd like to merge this.
There is a failing check in Travis (not related to this PR):

Tests in error:
JobManagerFailsITCase.run:40->org$scalatest$BeforeAndAfterAll$$super$run:40->org$scalatest$WordSpecLike$$super$run:40->runTests:40->runTest:40->withFixture:40 » Timeout
JobManagerFailsITCase.run:40->org$scalatest$BeforeAndAfterAll$$super$run:40->org$scalatest$WordSpecLike$$super$run:40->runTests:40->runTest:40->withFixture:40->TestKit.within:707->TestKit.within:707 » Timeout

Is this fixed by #422 #422? Shall I
proceed?
Thanks!


Reply to this email directly or view it on GitHub
#402 (comment).

@vasia
Copy link
Contributor

vasia commented Feb 23, 2015

Thank you @tillrohrmann! Then, I'll go ahead and merge this :)

@asfgit asfgit closed this in c111444 Feb 23, 2015
marthavk pushed a commit to marthavk/flink that referenced this pull request Jun 9, 2015
…y numberOfVertices() to read the number of vertices instead of passing it to the constructor.

This closes apache#402
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants