> On March 18, 2014, 7:48 p.m., kturner wrote: > > core/src/test/java/org/apache/accumulo/core/util/StatTest.java, line 16 > > <https://reviews.apache.org/r/19368/diff/1/?file=526386#file526386line16> > > > > maybe have a data set w/ single data point. > > Sean Busbey wrote: > a data set with an even number of data points would also be good. > > Mike Drob wrote: > If we're switching to using commons-math, I'd prefer to generally trust > their implementation. > > Sean Busbey wrote: > The advantage of good test coverage for Stat is that we need not be > concerned with wether or not commons-math is used under the hood. It also > helps verify that the behavior didn't change by our current switch. > > kturner wrote: > An alternative would be to just completely remove the stat class in > master and use commons math directly. I do not know if this is a good > option, Mike what do you think? I have not looked at commons math, so I am > not sure if this would complicate Accumulo's code. > > Mike Drob wrote: > commons-math is pretty straightforward to use, especially for simple > things like one-dimentional values like min/max/etc. The advantage of using > our Stat class, is that I presume we want multiple metrics and commons-math > does not provide an elegant way to feed the same sequence into multiple > calculations.
if we are keeping Stat, then I agree w/ what Sean said about having unit test for it - kturner ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19368/#review37612 ----------------------------------------------------------- On March 18, 2014, 8:40 p.m., Mike Drob wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/19368/ > ----------------------------------------------------------- > > (Updated March 18, 2014, 8:40 p.m.) > > > Review request for accumulo. > > > Bugs: ACCUMULO-2494 > https://issues.apache.org/jira/browse/ACCUMULO-2494 > > > Repository: accumulo > > > Description > ------- > > ACCUMULO-2494 Delegate math to commons-math > > > Diffs > ----- > > core/src/main/java/org/apache/accumulo/core/util/Stat.java > e65265c6decde47ef229377653112a677fef8112 > core/src/test/java/org/apache/accumulo/core/util/StatTest.java PRE-CREATION > > Diff: https://reviews.apache.org/r/19368/diff/ > > > Testing > ------- > > Unit tests results compared with calculations from Wolfram Alpha. > > > Thanks, > > Mike Drob > >
