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 availability and the total number of vertices
should be *optional*. Otherwise, we are introducing unnecessary overhead for
the majority of cases, i.e. when these are not actually used by an algorithm.
- The edge direction option and the degrees / numOfVertices should be
*separate extensions* and not tight together, i.e. we should allow a user to
choose the edge direction option independently of whether they also want to
access the degrees. To be consistent with the rest of the
VertexCentricIteration options, we could introduce methods
`setMessagingDirection()`, `setInDegreesAvailable()` and so on.
- The change should be transparent to user code, i.e. if a user has turned
on the option for accessing inDegrees, they should be able to call
`vertex.getInDegree()` inside the messaging/update functions, but
`vertex.getValue()` should still return the vertex value. In other words, this
Tuple3 you're using to store the degrees should be hidden from the public
methods.
- Finally, we should add documentation for these options, including
description of how they work and some illustrative examples.
Let me know what you think about the above and whether you have any
questions!
---
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 enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
