> On March 20, 2013, 10:56 p.m., Alessandro Presta wrote:
> > giraph-core/src/main/java/org/apache/giraph/utils/InMemoryVertexInput.java, 
> > line 129
> > <https://reviews.apache.org/r/10052/diff/1/?file=272631#file272631line129>
> >
> >     I would call this "getEdges", maybe "createEdges". Also, looks like it 
> > could be converted to a static method.

I changed the name, but if I make it static, I can't use the generic types.


> On March 20, 2013, 10:56 p.m., Alessandro Presta wrote:
> > giraph-core/src/main/java/org/apache/giraph/utils/TestGraph.java, line 42
> > <https://reviews.apache.org/r/10052/diff/1/?file=272634#file272634line42>
> >
> >     I think we should be able to skip this intermediate representation, and 
> > directly store the vertices in a Map<I, Vertex>.
> >     Then, InMemoryVertexInput can simply return references to the 
> > already-created vertices in TestGraph.
> >     No output format will be needed then, as the vertices will be modified 
> > in place.
> >     That way InternalVertexRunner#run() can simply return the same 
> > TestGraph reference, and a user can do any checks directly against that.

That sounds right, but given time constraints and the fact that the current 
implementation works, I'll leave it as it is for now.


> On March 20, 2013, 10:56 p.m., Alessandro Presta wrote:
> > giraph-core/src/main/java/org/apache/giraph/utils/InMemoryVertexInput.java, 
> > line 170
> > <https://reviews.apache.org/r/10052/diff/1/?file=272631#file272631line170>
> >
> >     Assigning the format's conf from the reader doesn't feel right.
> >     You can instead make the format implement 
> > ImmutableClassesGiraphConfigurable, so that Giraph will provide it with the 
> > conf upon instantiation.

I was going by the way that this is done in io.formats.TextVertexInputFormat 
(line 101). I can change it if you think it's better.


- Veselin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10052/#review18177
-----------------------------------------------------------


On March 20, 2013, 10:05 p.m., Veselin Stoyanov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10052/
> -----------------------------------------------------------
> 
> (Updated March 20, 2013, 10:05 p.m.)
> 
> 
> Review request for giraph.
> 
> 
> Description
> -------
> 
> A framework for running small test graphs in memory.
> 
> 
> Diffs
> -----
> 
>   giraph-core/src/main/java/org/apache/giraph/utils/InMemoryEdgeInput.java 
> PRE-CREATION 
>   giraph-core/src/main/java/org/apache/giraph/utils/InMemoryVertexInput.java 
> PRE-CREATION 
>   giraph-core/src/main/java/org/apache/giraph/utils/InMemoryVertexOutput.java 
> PRE-CREATION 
>   giraph-core/src/main/java/org/apache/giraph/utils/InternalVertexRunner.java 
> 7e0b955998a5f1890912819fa2ca74fbbf46fedd 
>   giraph-core/src/main/java/org/apache/giraph/utils/TestGraph.java 
> PRE-CREATION 
>   
> giraph-examples/src/test/java/org/apache/giraph/examples/ConnectedComponentsVertexTestInMemory.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/10052/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Veselin Stoyanov
> 
>

Reply via email to