----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20047/#review40835 -----------------------------------------------------------
3rdparty/libprocess/include/process/statistics.hpp <https://reviews.apache.org/r/20047/#comment74042> At this point how about making 'Statistics' a struct and just exposing these as members? Now that there is no logic within a 'Statistics' object.. 3rdparty/libprocess/include/process/statistics.hpp <https://reviews.apache.org/r/20047/#comment74051> newline? 3rdparty/libprocess/include/process/statistics.hpp <https://reviews.apache.org/r/20047/#comment74043> How about 'front()' and 'back()' here? 3rdparty/libprocess/include/process/statistics.hpp <https://reviews.apache.org/r/20047/#comment74044> How about 'front()' and 'back()' here? 3rdparty/libprocess/include/process/statistics.hpp <https://reviews.apache.org/r/20047/#comment74052> How about s/calculate_percentile/percentile/ and s/p/percentile_/? Can you also document the fact that we're expecting [0..1]? 3rdparty/libprocess/include/process/statistics.hpp <https://reviews.apache.org/r/20047/#comment74054> It seems like 'baseIndex' is the actual 'index', so s/baseIndex/index/? For 'index' I'm not sure, maybe 'position'? Would it be beneficial to mention that we're doing linear interpolation here to guide the reader? Last thing, flipping the multiplication on line 96 seems a bit easier to read: const double index = p * (values.size() - 1); 3rdparty/libprocess/include/process/statistics.hpp <https://reviews.apache.org/r/20047/#comment74053> Now that we're normalizing 'p', would it be nice to add some CHECKs that we're not going out of bounds on the vector? If we were normalizing 'baseIndex' I would say the CHECKs are not needed but the math obscures our expected 'baseIndex' range. 3rdparty/libprocess/include/process/statistics.hpp <https://reviews.apache.org/r/20047/#comment74056> Sounds good! 3rdparty/libprocess/src/process.cpp <https://reviews.apache.org/r/20047/#comment74055> Feel free to just remove this. - Ben Mahler On April 18, 2014, 11:24 p.m., Dominic Hamon wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/20047/ > ----------------------------------------------------------- > > (Updated April 18, 2014, 11:24 p.m.) > > > Review request for mesos, Benjamin Hindman and Ben Mahler. > > > Bugs: MESOS-1036 > https://issues.apache.org/jira/browse/MESOS-1036 > > > Repository: mesos-git > > > Description > ------- > > see summary > > > Diffs > ----- > > 3rdparty/libprocess/Makefile.am d707ad759dacd16e0177e14f1bf5ece9e4ce2491 > 3rdparty/libprocess/include/process/statistics.hpp > a4f1db3a8a219c39193a1d237477f0350e47e681 > 3rdparty/libprocess/src/process.cpp > 9654c0437edb43cff65dbefdf08dee9e18ef96ab > 3rdparty/libprocess/src/statistics.cpp > 75aac4074d33cb5054da6c8b0bd4a890c2eaf80e > 3rdparty/libprocess/src/tests/statistics_tests.cpp > 3521bd565dae8fcbba464f2539b3b14a37a037f0 > 3rdparty/libprocess/src/timeseries.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/20047/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Dominic Hamon > >
