[jira] [Commented] (GIRAPH-74) GIRAPH-36 missed a couple of liners when passing from Vertex to BasicVertex
[ https://issues.apache.org/jira/browse/GIRAPH-74?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13150724#comment-13150724 ] Claudio Martella commented on GIRAPH-74: Apparently I'm not. I cannot reach once again SVN (by the way is it me or is it normal? it happens often). If you have changed the API accordingly, i'll check those Vertex that should be Basic Vertex that were missing before. But from your message it looks like I'll need to change the field to a package-level set/get methods as well. > GIRAPH-36 missed a couple of liners when passing from Vertex to BasicVertex > --- > > Key: GIRAPH-74 > URL: https://issues.apache.org/jira/browse/GIRAPH-74 > Project: Giraph > Issue Type: Bug > Components: graph >Affects Versions: 0.70.0 >Reporter: Claudio Martella >Assignee: Claudio Martella > Fix For: 0.70.0 > > Attachments: GIRAPH-36-cleaup.diff, GIRAPH-70.diff > > > After GIRAPH-36, in a couple of places Vertex was left instead of > BasicVertex. To achieve the fix a BasicVertex was extended with a new boolean > isHalted(boolean state) method that totally hides boolean halt field. I > believe this unifies the voteToHalt() and isHalted() methods totally hiding > the halting management of Vertices (before it was relying on the existence of > boolean halt field). -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (GIRAPH-74) GIRAPH-36 missed a couple of liners when passing from Vertex to BasicVertex
[ https://issues.apache.org/jira/browse/GIRAPH-74?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13150663#comment-13150663 ] Avery Ching commented on GIRAPH-74: --- Claudio, are you synced to svn? The commit of GIRAPH-11 should have partially resolved your issues (saved the halted boolean in BasicVertex). I prefer the methods instead of a field as well. > GIRAPH-36 missed a couple of liners when passing from Vertex to BasicVertex > --- > > Key: GIRAPH-74 > URL: https://issues.apache.org/jira/browse/GIRAPH-74 > Project: Giraph > Issue Type: Bug > Components: graph >Affects Versions: 0.70.0 >Reporter: Claudio Martella >Assignee: Claudio Martella > Fix For: 0.70.0 > > Attachments: GIRAPH-36-cleaup.diff, GIRAPH-70.diff > > > After GIRAPH-36, in a couple of places Vertex was left instead of > BasicVertex. To achieve the fix a BasicVertex was extended with a new boolean > isHalted(boolean state) method that totally hides boolean halt field. I > believe this unifies the voteToHalt() and isHalted() methods totally hiding > the halting management of Vertices (before it was relying on the existence of > boolean halt field). -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (GIRAPH-74) GIRAPH-36 missed a couple of liners when passing from Vertex to BasicVertex
[ https://issues.apache.org/jira/browse/GIRAPH-74?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13150509#comment-13150509 ] Claudio Martella commented on GIRAPH-74: I agree with you to this extent, I synched it to the others willing to drop get/set methods. I could refactor to the return value of compute, but I'm afraid it would break the things here and there. We would still have to keep the state of the vertex somewhere... My favorite would be to have package-level set/get. > GIRAPH-36 missed a couple of liners when passing from Vertex to BasicVertex > --- > > Key: GIRAPH-74 > URL: https://issues.apache.org/jira/browse/GIRAPH-74 > Project: Giraph > Issue Type: Bug > Components: graph >Affects Versions: 0.70.0 >Reporter: Claudio Martella >Assignee: Claudio Martella > Fix For: 0.70.0 > > Attachments: GIRAPH-36-cleaup.diff, GIRAPH-70.diff > > > After GIRAPH-36, in a couple of places Vertex was left instead of > BasicVertex. To achieve the fix a BasicVertex was extended with a new boolean > isHalted(boolean state) method that totally hides boolean halt field. I > believe this unifies the voteToHalt() and isHalted() methods totally hiding > the halting management of Vertices (before it was relying on the existence of > boolean halt field). -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (GIRAPH-74) GIRAPH-36 missed a couple of liners when passing from Vertex to BasicVertex
[ https://issues.apache.org/jira/browse/GIRAPH-74?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13150498#comment-13150498 ] Gianmarco De Francisci Morales commented on GIRAPH-74: -- Just my 2 cents. Having a field instead of a method is generally a bad thing. If you want to later change this, you need to break the API. I personally like the idea to use the return value of compute instead of having a separate halting procedure, it makes the whole process look more "functional". > GIRAPH-36 missed a couple of liners when passing from Vertex to BasicVertex > --- > > Key: GIRAPH-74 > URL: https://issues.apache.org/jira/browse/GIRAPH-74 > Project: Giraph > Issue Type: Bug > Components: graph >Affects Versions: 0.70.0 >Reporter: Claudio Martella >Assignee: Claudio Martella > Fix For: 0.70.0 > > Attachments: GIRAPH-36-cleaup.diff, GIRAPH-70.diff > > > After GIRAPH-36, in a couple of places Vertex was left instead of > BasicVertex. To achieve the fix a BasicVertex was extended with a new boolean > isHalted(boolean state) method that totally hides boolean halt field. I > believe this unifies the voteToHalt() and isHalted() methods totally hiding > the halting management of Vertices (before it was relying on the existence of > boolean halt field). -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (GIRAPH-74) GIRAPH-36 missed a couple of liners when passing from Vertex to BasicVertex
[ https://issues.apache.org/jira/browse/GIRAPH-74?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13148080#comment-13148080 ] Claudio Martella commented on GIRAPH-74: I've always noted an ambiguity in the API. There's the halt boolean field which is there and is set by voteToHalt(), but then there's the isHalted() which can potentially lead to a conflictual implementation of the concept (the user might not know about the private field halt). So I propose: we either put the setHalted() to BasicVertex and give an implementation to Vertex so the user can ignore it, or we get rid of both isHalted and setHalted together. I don't see why we should also get rid of voteToHalt(), it has a strong semantical meaning and matches the original Pregel API. I agree though that the return value of compute might do that as well. So: I vote for getting rid of isHalted and setHalted. > GIRAPH-36 missed a couple of liners when passing from Vertex to BasicVertex > --- > > Key: GIRAPH-74 > URL: https://issues.apache.org/jira/browse/GIRAPH-74 > Project: Giraph > Issue Type: Bug > Components: graph >Affects Versions: 0.70.0 >Reporter: Claudio Martella >Assignee: Claudio Martella > Fix For: 0.70.0 > > Attachments: GIRAPH-36-cleaup.diff > > > After GIRAPH-36, in a couple of places Vertex was left instead of > BasicVertex. To achieve the fix a BasicVertex was extended with a new boolean > isHalted(boolean state) method that totally hides boolean halt field. I > believe this unifies the voteToHalt() and isHalted() methods totally hiding > the halting management of Vertices (before it was relying on the existence of > boolean halt field). -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (GIRAPH-74) GIRAPH-36 missed a couple of liners when passing from Vertex to BasicVertex
[ https://issues.apache.org/jira/browse/GIRAPH-74?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13147975#comment-13147975 ] Avery Ching commented on GIRAPH-74: --- Well, if you vote to halt, your compute() is never run again (unless you get a message and your state is reset at a later superstep). The halted state after compute() is called is the one that is final for the vertex for that particular superstep. Another way of doing this could be to have compute return a boolean and get rid of setHalted() and voteToHalt(). > GIRAPH-36 missed a couple of liners when passing from Vertex to BasicVertex > --- > > Key: GIRAPH-74 > URL: https://issues.apache.org/jira/browse/GIRAPH-74 > Project: Giraph > Issue Type: Bug > Components: graph >Affects Versions: 0.70.0 >Reporter: Claudio Martella >Assignee: Claudio Martella > Fix For: 0.70.0 > > Attachments: GIRAPH-36-cleaup.diff > > > After GIRAPH-36, in a couple of places Vertex was left instead of > BasicVertex. To achieve the fix a BasicVertex was extended with a new boolean > isHalted(boolean state) method that totally hides boolean halt field. I > believe this unifies the voteToHalt() and isHalted() methods totally hiding > the halting management of Vertices (before it was relying on the existence of > boolean halt field). -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (GIRAPH-74) GIRAPH-36 missed a couple of liners when passing from Vertex to BasicVertex
[ https://issues.apache.org/jira/browse/GIRAPH-74?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13147972#comment-13147972 ] Jake Mannix commented on GIRAPH-74: --- But my concern is regarding how this works with general BSP algorithm correctness: if you can vote to halt, then later cancel it, can this lead to problems, or is this generally allowed? > GIRAPH-36 missed a couple of liners when passing from Vertex to BasicVertex > --- > > Key: GIRAPH-74 > URL: https://issues.apache.org/jira/browse/GIRAPH-74 > Project: Giraph > Issue Type: Bug > Components: graph >Affects Versions: 0.70.0 >Reporter: Claudio Martella >Assignee: Claudio Martella > Fix For: 0.70.0 > > Attachments: GIRAPH-36-cleaup.diff > > > After GIRAPH-36, in a couple of places Vertex was left instead of > BasicVertex. To achieve the fix a BasicVertex was extended with a new boolean > isHalted(boolean state) method that totally hides boolean halt field. I > believe this unifies the voteToHalt() and isHalted() methods totally hiding > the halting management of Vertices (before it was relying on the existence of > boolean halt field). -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (GIRAPH-74) GIRAPH-36 missed a couple of liners when passing from Vertex to BasicVertex
[ https://issues.apache.org/jira/browse/GIRAPH-74?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13147968#comment-13147968 ] Avery Ching commented on GIRAPH-74: --- Either the vertex will keep track of it or the user will (have their own boolean and then decide whether to vote to halt or not). I don't have a strong opinion. > GIRAPH-36 missed a couple of liners when passing from Vertex to BasicVertex > --- > > Key: GIRAPH-74 > URL: https://issues.apache.org/jira/browse/GIRAPH-74 > Project: Giraph > Issue Type: Bug > Components: graph >Affects Versions: 0.70.0 >Reporter: Claudio Martella >Assignee: Claudio Martella > Fix For: 0.70.0 > > Attachments: GIRAPH-36-cleaup.diff > > > After GIRAPH-36, in a couple of places Vertex was left instead of > BasicVertex. To achieve the fix a BasicVertex was extended with a new boolean > isHalted(boolean state) method that totally hides boolean halt field. I > believe this unifies the voteToHalt() and isHalted() methods totally hiding > the halting management of Vertices (before it was relying on the existence of > boolean halt field). -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (GIRAPH-74) GIRAPH-36 missed a couple of liners when passing from Vertex to BasicVertex
[ https://issues.apache.org/jira/browse/GIRAPH-74?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13147948#comment-13147948 ] Jake Mannix commented on GIRAPH-74: --- In general, I'd prefer setHalted(boolean), but this allows a Vertex to effectively cancel its vote to halt at a later time. Should that be allowed? > GIRAPH-36 missed a couple of liners when passing from Vertex to BasicVertex > --- > > Key: GIRAPH-74 > URL: https://issues.apache.org/jira/browse/GIRAPH-74 > Project: Giraph > Issue Type: Bug > Components: graph >Affects Versions: 0.70.0 >Reporter: Claudio Martella >Assignee: Claudio Martella > Fix For: 0.70.0 > > Attachments: GIRAPH-36-cleaup.diff > > > After GIRAPH-36, in a couple of places Vertex was left instead of > BasicVertex. To achieve the fix a BasicVertex was extended with a new boolean > isHalted(boolean state) method that totally hides boolean halt field. I > believe this unifies the voteToHalt() and isHalted() methods totally hiding > the halting management of Vertices (before it was relying on the existence of > boolean halt field). -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (GIRAPH-74) GIRAPH-36 missed a couple of liners when passing from Vertex to BasicVertex
[ https://issues.apache.org/jira/browse/GIRAPH-74?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13147919#comment-13147919 ] Avery Ching commented on GIRAPH-74: --- Some of this I also addressed in GIRAPH-11 =). I think combining the getter/setter behavior is a little strange. Perhaps void setHalted(boolean isHalted); boolean isHalted(); Anyone else have an opinion? > GIRAPH-36 missed a couple of liners when passing from Vertex to BasicVertex > --- > > Key: GIRAPH-74 > URL: https://issues.apache.org/jira/browse/GIRAPH-74 > Project: Giraph > Issue Type: Bug > Components: graph >Affects Versions: 0.70.0 >Reporter: Claudio Martella >Assignee: Claudio Martella > Fix For: 0.70.0 > > Attachments: GIRAPH-36-cleaup.diff > > > After GIRAPH-36, in a couple of places Vertex was left instead of > BasicVertex. To achieve the fix a BasicVertex was extended with a new boolean > isHalted(boolean state) method that totally hides boolean halt field. I > believe this unifies the voteToHalt() and isHalted() methods totally hiding > the halting management of Vertices (before it was relying on the existence of > boolean halt field). -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
