> On 2011-12-08 07:04:49, Ted Dunning wrote: > > trunk/core/src/main/java/org/apache/mahout/classifier/sgd/mapreduce/PassiveAggressiveMapper.java, > > line 36 > > <https://reviews.apache.org/r/3072/diff/2/?file=63195#file63195line36> > > > > Needs a comment about how this works.
Added comments. > On 2011-12-08 07:04:49, Ted Dunning wrote: > > trunk/core/src/main/java/org/apache/mahout/classifier/sgd/mapreduce/PassiveAggressiveMapper.java, > > lines 67-75 > > <https://reviews.apache.org/r/3072/diff/2/?file=63195#file63195line67> > > > > This really need a comment. What is the purpose here? Added comments. > On 2011-12-08 07:04:49, Ted Dunning wrote: > > trunk/core/src/main/java/org/apache/mahout/classifier/sgd/mapreduce/PassiveAggressiveMapper.java, > > lines 98-111 > > <https://reviews.apache.org/r/3072/diff/2/?file=63195#file63195line98> > > > > What is this intended to do? Why? Added comments. > On 2011-12-08 07:04:49, Ted Dunning wrote: > > trunk/core/src/main/java/org/apache/mahout/classifier/sgd/mapreduce/PassiveAggressiveReducer.java, > > line 30 > > <https://reviews.apache.org/r/3072/diff/2/?file=63196#file63196line30> > > > > Typo. > > > > Also, this doesn't say how this works or why it is the way it is. Fixed the typo and added comments. > On 2011-12-08 07:04:49, Ted Dunning wrote: > > trunk/core/src/main/java/org/apache/mahout/classifier/sgd/mapreduce/PassiveAggressiveReducer.java, > > line 32 > > <https://reviews.apache.org/r/3072/diff/2/?file=63196#file63196line32> > > > > Shouldn't there be a combiner as well? A combiner isn't needed because each map task submits one value overall. > On 2011-12-08 07:04:49, Ted Dunning wrote: > > trunk/core/src/main/java/org/apache/mahout/classifier/sgd/mapreduce/PassiveAggressiveReducer.java, > > line 53 > > <https://reviews.apache.org/r/3072/diff/2/?file=63196#file63196line53> > > > > A comment here about what this weight is would be nice. Also, how can > > a double be a key? That is tantamount to comparing doubles which is bad. Added comments. it is not the weight of the classifier but the weight of the weighted average. > On 2011-12-08 07:04:49, Ted Dunning wrote: > > trunk/core/src/main/java/org/apache/mahout/classifier/sgd/mapreduce/SGDDriver.java, > > line 99 > > <https://reviews.apache.org/r/3072/diff/2/?file=63197#file63197line99> > > > > Where does the InterruptedException come from? It comes from runIteration function. > On 2011-12-08 07:04:49, Ted Dunning wrote: > > trunk/core/src/main/java/org/apache/mahout/classifier/sgd/mapreduce/SGDDriver.java, > > lines 110-111 > > <https://reviews.apache.org/r/3072/diff/2/?file=63197#file63197line110> > > > > Use brackets Added brackets. > On 2011-12-08 07:04:49, Ted Dunning wrote: > > trunk/core/src/test/java/org/apache/mahout/classifier/sgd/mapreduce/AdaptiveLogisticRegressionMapReduceTest.java, > > line 35 > > <https://reviews.apache.org/r/3072/diff/2/?file=63198#file63198line35> > > > > Should not throw Exception Added IO Exception and Interrupted Exception. > On 2011-12-08 07:04:49, Ted Dunning wrote: > > trunk/core/src/main/java/org/apache/mahout/classifier/sgd/mapreduce/PassiveAggressiveMapper.java, > > lines 53-56 > > <https://reviews.apache.org/r/3072/diff/2/?file=63195#file63195line53> > > > > This is nearly duplicated code. The mapper and reducer should share > > some code to avoid inconsistent defaults. Created a base class which shares the same initialization code. - issei ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3072/#review3734 ----------------------------------------------------------- On 2011-12-12 11:51:59, issei yoshida wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/3072/ > ----------------------------------------------------------- > > (Updated 2011-12-12 11:51:59) > > > Review request for mahout. > > > Summary > ------- > > MAHOUT-918 Parallelized SGD in MapReduce > > > This addresses bug MAHOUT-918. > https://issues.apache.org/jira/browse/MAHOUT-918 > > > Diffs > ----- > > > trunk/core/src/main/java/org/apache/mahout/classifier/sgd/PassiveAggressive.java > 1213193 > > trunk/core/src/main/java/org/apache/mahout/classifier/sgd/mapreduce/AdaptiveLogisticRegressionDriver.java > PRE-CREATION > > trunk/core/src/main/java/org/apache/mahout/classifier/sgd/mapreduce/AdaptiveLogisticRegressionMapper.java > PRE-CREATION > > trunk/core/src/main/java/org/apache/mahout/classifier/sgd/mapreduce/AdaptiveLogisticRegressionReducer.java > PRE-CREATION > > trunk/core/src/main/java/org/apache/mahout/classifier/sgd/mapreduce/LogisticRegressionDriver.java > PRE-CREATION > > trunk/core/src/main/java/org/apache/mahout/classifier/sgd/mapreduce/LogisticRegressionMapper.java > PRE-CREATION > > trunk/core/src/main/java/org/apache/mahout/classifier/sgd/mapreduce/LogisticRegressionReducer.java > PRE-CREATION > > trunk/core/src/main/java/org/apache/mahout/classifier/sgd/mapreduce/PassiveAggressiveDriver.java > PRE-CREATION > > trunk/core/src/main/java/org/apache/mahout/classifier/sgd/mapreduce/PassiveAggressiveMapper.java > PRE-CREATION > > trunk/core/src/main/java/org/apache/mahout/classifier/sgd/mapreduce/PassiveAggressiveReducer.java > PRE-CREATION > > trunk/core/src/main/java/org/apache/mahout/classifier/sgd/mapreduce/SGDDriver.java > PRE-CREATION > > trunk/core/src/main/java/org/apache/mahout/classifier/sgd/mapreduce/SGDMapper.java > PRE-CREATION > > trunk/core/src/main/java/org/apache/mahout/classifier/sgd/mapreduce/SGDReducer.java > PRE-CREATION > > trunk/core/src/test/java/org/apache/mahout/classifier/sgd/mapreduce/AdaptiveLogisticRegressionMapReduceTest.java > PRE-CREATION > > trunk/core/src/test/java/org/apache/mahout/classifier/sgd/mapreduce/LogisticRegressionMapReduceTest.java > PRE-CREATION > > trunk/core/src/test/java/org/apache/mahout/classifier/sgd/mapreduce/PassiveAggressiveMapReduceTest.java > PRE-CREATION > > trunk/core/src/test/java/org/apache/mahout/classifier/sgd/mapreduce/SGDMapReduceTest.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/3072/diff > > > Testing > ------- > > > Thanks, > > issei > >
