Feature diffusion computation and utils#82
Feature diffusion computation and utils#82AlessioRosatelli wants to merge 5 commits intoapache:trunkfrom
Conversation
| @@ -0,0 +1,39 @@ | |||
| package org.apache.giraph.examples; | |||
There was a problem hiding this comment.
Let's use some consistent code style to format the sources. I suggest Google's java formatting: http://google.github.io/styleguide/javaguide.html. You can use it directly in Eclispe/IDEA and apply formatting changes automatically.
| @@ -0,0 +1,39 @@ | |||
| package org.apache.giraph.examples; | |||
|
|
|||
| import org.apache.giraph.block_app.framework.block.Block; | |||
There was a problem hiding this comment.
Similarly, let's have checkstyle (http://checkstyle.sourceforge.net/google_style.html) and findbugs (http://findbugs.sourceforge.net/) in place. This will greatly improve the readability of the code
| protected long label; | ||
| protected int treshold; | ||
| protected boolean changed = false; | ||
| protected double temp; |
There was a problem hiding this comment.
please use readable names for all variables/objects
| }*/ | ||
|
|
||
| @Override | ||
| public void compute(Vertex<LongWritable, LabelingVertexValue, NullWritable> vertex, Iterable<Text> msgs) |
There was a problem hiding this comment.
Let's send typed messages, e.g., LongLongWritable in this case.
| Logger LOG = Logger.getLogger(this.getClass()); | ||
|
|
||
|
|
||
| /*public void initialize(GraphState graphState, |
There was a problem hiding this comment.
Please get rid of all debug code. It is hard to read the code otherwise
| } | ||
|
|
||
| public boolean isVertexDead() { | ||
| return new BigDecimal(currentActivationProbability).setScale(2, RoundingMode.HALF_DOWN).floatValue() == 0; |
There was a problem hiding this comment.
I believe BigDecimal are not needed in this case
|
|
||
| protected int vertexThreshold; | ||
| protected int label; | ||
| protected double currentActivationProbability=0.2; |
There was a problem hiding this comment.
I'm confused. This constant is also initialized in DiffusionMigrationSimulationComputation. Please define constants in a single place
| private void setup(DiffusionVertexValue value) { | ||
| double delta = Double.parseDouble(getConf().getStrings("Delta", "0.005")[0]); | ||
| value.setDelta(delta); | ||
| double initialActivationProbability = Double.parseDouble(getConf().getStrings("InitialProbability","0.02")[0]); |
There was a problem hiding this comment.
Please use org.apache.giraph.conf.FloatConfOption instead
|
|
||
| import org.apache.giraph.examples.feature_diffusion_utils.datastructures.DiffusionVertexValue; | ||
|
|
||
| public class DiffusionMigrationSimulationComputation extends MigrationFullBasicComputation<LongWritable, DiffusionVertexValue, NullWritable, IntWritable> { |
There was a problem hiding this comment.
Please add comment for all your classes, methods, and constants
| import org.apache.giraph.examples.feature_diffusion_utils.datastructures.DiffusionVertexValue; | ||
|
|
||
|
|
||
| public class DiffusionMigrationBlockFactory extends MigrationFullBlockFactory { |
There was a problem hiding this comment.
Let's add a few unit tests to make sure things are working as expected
3 participants
No description provided.