Added KernelSegmenter. #3590

samuelklee · 2017-09-19T17:46:49Z

Some of the posts in #2858 might be helpful in reviewing this PR.

samuelklee · 2017-09-19T17:52:50Z

@davidbenjamin Feel free to comment if you like, but @mbabadi will handle the main review. Thanks guys!

davidbenjamin

A few comments but I'm reading for my own benefit, with the review as a side effect.

davidbenjamin · 2017-09-19T18:50:44Z

.../java/org/broadinstitute/hellbender/tools/copynumber/utils/segmentation/KernelSegmenter.java

+                                          final List<Integer> windowSizes,
+                                          final double numChangepointsPenaltyLinearFactor,
+                                          final double numChangepointsPenaltyLogLinearFactor,
+                                          final boolean sortByIndex) {


Consider implementing this as a nested enum:

public enum ChangepointSortOrder { SORT_BY_INDEX, SORT_BY_BACKWARD_SELECTION }

davidbenjamin · 2017-09-19T18:56:04Z

.../java/org/broadinstitute/hellbender/tools/copynumber/utils/segmentation/KernelSegmenter.java

+        ParamUtils.isPositiveOrZero(maxNumChangepoints, "Maximum number of changepoints must be non-negative.");
+        ParamUtils.isPositive(kernelApproximationDimension, "Dimension of kernel approximation must be positive.");
+        Utils.validateArg(windowSizes.stream().allMatch(ws -> ws > 0), "Window sizes must all be positive.");
+        Utils.validateArg(new HashSet<>(windowSizes).size() == windowSizes.size(), "Window sizes must all be unique.");


windowSizes.stream().distinct().count() == windowSizes.size() is slightly wordier but more direct.

davidbenjamin · 2017-09-19T19:20:36Z

.../java/org/broadinstitute/hellbender/tools/copynumber/utils/segmentation/KernelSegmenter.java

+            final double[] invSqrtSingularValues = Arrays.stream(svd.getSingularValues()).map(Math::sqrt).map(x -> 1. / (x + EPSILON)).toArray();
+            @Override
+            public double visit(int i, int j, double value) {
+                double sum = 0.;


return new IndexRange(0, numSubsample).sum(k -> kernel.apply(data.get(i), dataSubsample.get(k)) * svd.getU().getEntry(k, j) * invSqrtSingularValues[j])

davidbenjamin · 2017-09-19T19:28:54Z

.../java/org/broadinstitute/hellbender/tools/copynumber/utils/segmentation/KernelSegmenter.java

+    //for N x p matrix Z_ij, returns the N-dimensional vector sum(Z_ij * Z_ij, j = 0,..., p - 1),
+    //which are the diagonal elements K_ii of the approximate kernel matrix
+    private static double[] calculateKernelApproximationDiagonal(final RealMatrix reducedObservationMatrix) {
+        return IntStream.range(0, reducedObservationMatrix.getRowDimension()).boxed()


It's lighter and shorter to use IndexRange:

return new IndexRange(0, reducedObservationMatrix.getRowDimension()) .mapToDouble(i -> Arrays.stream(reducedObservationMatrix.getRow(i)).map(z -> z * z).sum());

You could go a step further with

return new IndexRange(0, reducedObservationMatrix.getRowDimension()) .mapToDouble(i -> MathUtils.square(reducedObservationMatrix.getRowVector(i).getNorm()));

davidbenjamin · 2017-09-19T19:37:20Z

.../java/org/broadinstitute/hellbender/tools/copynumber/utils/segmentation/KernelSegmenter.java

+        //calculate penalties as a function of the number of changepoints
+        final int numData = reducedObservationMatrix.getRowDimension();
+        final List<Double> changepointPenalties = IntStream.range(0, maxNumChangepoints + 1).boxed()
+                .map(numChangepoints -> numChangepointsPenaltyLinearFactor * numChangepoints


It's a one-liner, but I nonetheless think it's worth extracting a method for the changepoint penalty.

Agreed, done.

davidbenjamin · 2017-09-19T19:39:22Z

.../java/org/broadinstitute/hellbender/tools/copynumber/utils/segmentation/KernelSegmenter.java

+                .collect(Collectors.toList());
+
+        //construct initial list of all segments and initialize costs
+        final List<Integer> candidateStarts = changepointCandidates.stream().sorted().distinct()


How about combine starts and ends into a List<IndexRange> candidate segments, which could be created from a single stream?

Segments as I've defined them are end inclusive, but IndexRange is supposed to represent an end exclusive interval, so I'll leave things as they are.

davidbenjamin · 2017-09-19T19:41:42Z

.../java/org/broadinstitute/hellbender/tools/copynumber/utils/segmentation/KernelSegmenter.java

+        final List<Double> costsForMergedSegmentPairs = IntStream.range(0, numSegments - 1).boxed()
+                .map(i -> new Segment(candidateStarts.get(i), candidateEnds.get(i + 1), reducedObservationMatrix, kernelApproximationDiagonal).cost)
+                .collect(Collectors.toList());  //cost of each adjacent pair when considered as a single segment
+        final List<Double> costsForMergingSegmentPairs = IntStream.range(0, numSegments - 1).boxed()


If you use double[] above this reduces to MathArrays.ebeSubtract.

Good point. However, I'm removing elements from the lists, so I'd rather use an ArrayList (since I like to use arrays to represent fixed-length quantities).

davidbenjamin · 2017-09-19T19:54:40Z

.../java/org/broadinstitute/hellbender/tools/copynumber/utils/segmentation/KernelSegmenter.java

+        //initialize quantities for recurrence
+        double D = kernelApproximationDiagonal[start];
+        final double[] W = Arrays.copyOf(reducedObservationMatrix.getRow(start), p);
+        double V = Arrays.stream(W).map(w -> w * w).sum();


Might be worth extracting a MathUtils for sum of squares of a double[].

Where are the boy scouts of yesteryear...

davidbenjamin · 2017-09-19T19:57:41Z

.../java/org/broadinstitute/hellbender/tools/copynumber/utils/segmentation/KernelSegmenter.java

+            //update quantities in left segment
+            leftD -= kernelApproximationDiagonal[start];
+            ZdotW = 0.;
+            for (int j = 0; j < p; j++) {


Can you extract a method for this four-line motif?

I thought about it, but decided against it because such a method needs to either return ZdotW and modify the W costs as a side effect or return a pair of quantities. There are also enough variations on the motif to require parameterization of the method. But it does shorten up the code a bit, so done!

samuelklee · 2017-09-20T15:29:23Z

Whoops, just realized I broke the tests with some changes in a previous PR. Will fix and address @davidbenjamin's comments, thanks!

codecov-io · 2017-09-20T17:01:21Z

Codecov Report

Merging #3590 into master will increase coverage by 0.358%.
The diff coverage is 93.939%.

@@               Coverage Diff               @@
##              master     #3590       +/-   ##
===============================================
+ Coverage     79.736%   80.094%   +0.358%     
- Complexity     18148     18799      +651     
===============================================
  Files           1217      1226        +9     
  Lines          66602     69015     +2413     
  Branches       10429     11073      +644     
===============================================
+ Hits           53106     55277     +2171     
- Misses          9289      9415      +126     
- Partials        4207      4323      +116

Impacted Files	Coverage Δ	Complexity Δ
...umber/utils/optimization/PersistenceOptimizer.java	`84.946% <ø> (ø)`	`27 <0> (ø)`	⬇️
...copynumber/utils/segmentation/KernelSegmenter.java	`93.939% <93.939%> (ø)`	`44 <44> (?)`
.../tools/spark/sv/evidence/QNamesForKmersFinder.java	`83.333% <0%> (-16.667%)`	`7% <0%> (ø)`
...nder/tools/spark/pathseq/PSPathogenTaxonScore.java	`78.125% <0%> (-10.11%)`	`20% <0%> (+17%)`
.../copynumber/allelic/alleliccount/AllelicCount.java	`65.854% <0%> (-2.003%)`	`19% <0%> (+6%)`
...te/hellbender/tools/spark/sv/utils/SVKmerizer.java	`85.714% <0%> (-1.786%)`	`18% <0%> (ø)`
...ools/spark/pathseq/PSFilterArgumentCollection.java	`79.487% <0%> (-1.158%)`	`3% <0%> (+1%)`
...ecaller/AssemblyBasedCallerArgumentCollection.java	`100% <0%> (ø)`	`2% <0%> (+1%)`	⬆️
...oadinstitute/hellbender/utils/gcs/BucketUtils.java	`78.571% <0%> (ø)`	`39% <0%> (ø)`	⬇️
...tools/spark/pathseq/PSScoreArgumentCollection.java	`100% <0%> (ø)`	`1% <0%> (ø)`	⬇️
... and 37 more

davidbenjamin · 2017-09-20T17:05:30Z

I'm satisfied.

…tion.

samuelklee · 2017-09-26T13:06:38Z

@mbabadi Can you review? I need to carry changes from this PR forward in the rest of the branch. Thanks!

mbabadi · 2017-09-26T16:22:16Z

I'm halfway through, will finish today :)

…

On Sep 26, 2017 9:14 AM, "samuelklee" ***@***.***> wrote: @mbabadi <https://github.com/mbabadi> Can you review? I need to carry changes from this PR forward in the rest of the branch. Thanks! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3590 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AOmMjb0_wGN9Bf1zIK2J3BmJybK7WmiAks5smPbkgaJpZM4PcxTR> .

mbabadi

Great PR! the code is well-organized and well-written :)

mbabadi · 2017-09-25T20:45:30Z

.../java/org/broadinstitute/hellbender/tools/copynumber/utils/segmentation/KernelSegmenter.java

+ *     This gives the global cost as a function of the number of changepoints <i>C</i>.
+ * </ol>
+ * <ol>
+ *     6) Add a penalty <i>A * C + B * C * log N / C</i> to the global cost and find the minimum to determine the


log N / C -> log(N / C) for clarity.

Also, perhaps add that T is the generic data type, and that this class works for segmenting generic sequential data so long as kernel (T, T) -> R is provided.

I guess you forgot to change all instances of T to DATA.

mbabadi · 2017-09-25T20:51:14Z

.../java/org/broadinstitute/hellbender/tools/copynumber/utils/segmentation/KernelSegmenter.java

+
+    private final List<T> data;
+
+    public KernelSegmenter(final List<T> data) {


What about making KernelSegmeter a fully static class? use a private constructor to prevent instantiation; data is only used in findChangepoints. It can have a public static declaration and take data as an additional argument.

I think I was allowing for the possibility of passing the data once and finding multiple sets of changepoints. This could also be done with a static method, I guess...not sure if there are any advantages/disadvantages either way, so I'll leave it as is, if you don't mind!

The advantage of the static implementation would be a small saving in instantiation space/time cost, but that's negligible for our current use cases; so I'm satisfied.

mbabadi · 2017-09-25T20:55:02Z

.../java/org/broadinstitute/hellbender/tools/copynumber/utils/segmentation/KernelSegmenter.java

+                                          final boolean sortByIndex) {
+        ParamUtils.isPositiveOrZero(maxNumChangepoints, "Maximum number of changepoints must be non-negative.");
+        ParamUtils.isPositive(kernelApproximationDimension, "Dimension of kernel approximation must be positive.");
+        Utils.validateArg(windowSizes.stream().allMatch(ws -> ws > 0), "Window sizes must all be positive.");


Also assert !windowSizes.isEmpty().

mbabadi · 2017-09-25T21:08:01Z

.../java/org/broadinstitute/hellbender/tools/copynumber/utils/segmentation/KernelSegmenter.java

+
+        //perform SVD of kernel matrix of subsampled data
+        logger.info(String.format("Performing SVD of kernel matrix of subsampled data (%d x %d)...", numSubsample, numSubsample));
+        final SingularValueDecomposition svd = new SingularValueDecomposition(subKernelMatrix);


I believe the SVD implementation in ojAlgo is substantially faster than Apache's. Also, you may want to consider using sgesvd from nd4j (http://nd4j.org/doc/org/nd4j/linalg/api/blas/Lapack.html) for more speed, if SVD is a computational bottleneck in your algorithm. The problem is that you are using Apache matrices so this solution requires adaptors between RealMatrix and INDArray. By the way, the garbage generation issues of nd4j has been solved in nd4j 0.9.0 using workspaces (see https://deeplearning4j.org/workspaces)

Actually, I think the bottleneck turned out to be the original non-matrix-multiplication implementation of this method.

I'll keep using Apache's SVD for now, but might consider implementing randomized SVD and using that here in the future.

mbabadi · 2017-09-25T21:15:13Z

.../java/org/broadinstitute/hellbender/tools/copynumber/utils/segmentation/KernelSegmenter.java

+        for (final int tauPrime : indices) {
+            D += kernelApproximationDiagonal[tauPrime];
+            final double ZdotW = calculateZdotWAndModifyW(reducedObservationMatrix, p, W, tauPrime, 1);
+            V += 2. * ZdotW + kernelApproximationDiagonal[tauPrime];


2. -> 2

I like making my doubles explicit...just a (bad?) habit!

mbabadi · 2017-09-25T21:25:31Z

.../java/org/broadinstitute/hellbender/tools/copynumber/utils/segmentation/KernelSegmenter.java

+            changepointCandidates.addAll(windowCostLocalMinima.subList(0, Math.min(maxNumChangepoints, windowCostLocalMinima.size())));
+        }
+
+        if (changepointCandidates.isEmpty()) {


Theoretically speaking, under what conditions changepointCandidates would end up being empty? is it always user error (bad window size, rank of the reduced data matrix being too low, empty data matrix, etc?). I don't understand the nuances of kernel segmentation, but I have a feeling that changepointCandidates should be non-empty under fairly lax conditions. Could you isolate the edge cases and throw more informative exceptions at early stages such that you would never encounter changepointCandidates.isEmpty() at this stage?

Yeah, good point. I think if you have non-empty data, you are always guaranteed a local minimum from PersistenceOptimizer, so I guess this check isn't necessary. I'll add a test for empty data and decide on the appropriate behavior in that case.

As for the unlikely case of a low-rank reduced data matrix or a bad SVD result, I think I'll cross my fingers and hope that appropriate exceptions are thrown from the SVD code...

Decided to add a warning and return an empty list in the case of no data.

but we still have UserException.BadInput?

mbabadi · 2017-09-26T17:03:11Z

.../java/org/broadinstitute/hellbender/tools/copynumber/utils/segmentation/KernelSegmenter.java

+                .collect(Collectors.toList());
+        final int numChangepointsOptimal = totalSegmentationCostsPlusPenalties.indexOf(Collections.min(totalSegmentationCostsPlusPenalties));
+
+        logger.info(String.format("Found %d changepoints after applying penalties.", numChangepointsOptimal));


"Found %d changepoints after applying the segmentation penalty."

mbabadi · 2017-09-26T17:06:16Z

.../java/org/broadinstitute/hellbender/tools/copynumber/utils/segmentation/KernelSegmenter.java

+        return windowCosts;
+    }
+
+    private static double calculateZdotWAndModifyW(final RealMatrix reducedObservationMatrix,


Could you add a brief description of the signMultiplier, and what does modifying W means?

In my opinion, extracting calculateZdotWAndModifyW has made the code less readable :)

Actually, I think I agree...I'll change it back if @davidbenjamin doesn't mind. (snip snap snip snap!)

mbabadi · 2017-09-26T17:10:36Z

...g/broadinstitute/hellbender/tools/copynumber/utils/segmentation/KernelSegmenterUnitTest.java

+ * @author Samuel Lee &lt;slee@broadinstitute.org&gt;
+ */
+public final class KernelSegmenterUnitTest extends BaseTest {
+    private static final int RANDOM_SEED = 1;   //reset seed before each simulated test case


//random seed is reset to this value before each simulated test case

This was meant to be an imperative to future developers...I'll make it more explicit (//make sure to reset random seed to this value before each simulated test case)

mbabadi · 2017-09-26T17:22:10Z

...g/broadinstitute/hellbender/tools/copynumber/utils/segmentation/KernelSegmenterUnitTest.java

+public final class KernelSegmenterUnitTest extends BaseTest {
+    private static final int RANDOM_SEED = 1;   //reset seed before each simulated test case
+
+    @DataProvider(name = "dataKernelSegmenter")


Could you briefly describe the two simulated test data in the unit test class javadoc (e.g. the first case is Gaussian data with discrete jumps in the mean every 100 points; the second case simulates a multimodal series such that the mean alternates in sign successively while the absolute value of the mean jumps every 100 points).

I would also include a few corner cases (empty data, empty window size, single-segment simulated data), and perhaps an example for a small but obvious changepoint + long wavelength sinusoidal background and how including small window sizes help detect it.

Good points. Added everything except the sinusoidal background case. Although useful, I think it's a bit beyond the scope of the unit test.

coughlazycough OK! ;-)

samuelklee · 2017-09-26T18:00:53Z

.../java/org/broadinstitute/hellbender/tools/copynumber/utils/segmentation/KernelSegmenter.java

+ *     This gives the global cost as a function of the number of changepoints <i>C</i>.
+ * </ol>
+ * <ol>
+ *     6) Add a penalty <i>A * C + B * C * log N / C</i> to the global cost and find the minimum to determine the


samuelklee · 2017-09-26T18:09:30Z

.../java/org/broadinstitute/hellbender/tools/copynumber/utils/segmentation/KernelSegmenter.java

+                                          final boolean sortByIndex) {
+        ParamUtils.isPositiveOrZero(maxNumChangepoints, "Maximum number of changepoints must be non-negative.");
+        ParamUtils.isPositive(kernelApproximationDimension, "Dimension of kernel approximation must be positive.");
+        Utils.validateArg(windowSizes.stream().allMatch(ws -> ws > 0), "Window sizes must all be positive.");


samuelklee · 2017-09-26T18:11:09Z

.../java/org/broadinstitute/hellbender/tools/copynumber/utils/segmentation/KernelSegmenter.java

+
+        //perform SVD of kernel matrix of subsampled data
+        logger.info(String.format("Performing SVD of kernel matrix of subsampled data (%d x %d)...", numSubsample, numSubsample));
+        final SingularValueDecomposition svd = new SingularValueDecomposition(subKernelMatrix);


Actually, I think the bottleneck turned out to be the original non-matrix-multiplication implementation of this method.

I'll keep using Apache's SVD for now, but might consider implementing randomized SVD and using that here in the future.

samuelklee · 2017-09-26T18:13:11Z

.../java/org/broadinstitute/hellbender/tools/copynumber/utils/segmentation/KernelSegmenter.java

+            changepointCandidates.addAll(windowCostLocalMinima.subList(0, Math.min(maxNumChangepoints, windowCostLocalMinima.size())));
+        }
+
+        if (changepointCandidates.isEmpty()) {


Yeah, good point. I think if you have non-empty data, you are always guaranteed a local minimum from PersistenceOptimizer, so I guess this check isn't necessary. I'll add a test for empty data and decide on the appropriate behavior in that case.

As for the unlikely case of a low-rank reduced data matrix or a bad SVD result, I think I'll cross my fingers and hope that appropriate exceptions are thrown from the SVD code...

samuelklee · 2017-09-26T18:13:41Z

.../java/org/broadinstitute/hellbender/tools/copynumber/utils/segmentation/KernelSegmenter.java

+                .collect(Collectors.toList());
+        final int numChangepointsOptimal = totalSegmentationCostsPlusPenalties.indexOf(Collections.min(totalSegmentationCostsPlusPenalties));
+
+        logger.info(String.format("Found %d changepoints after applying penalties.", numChangepointsOptimal));


Done (although I called it a changepoint penalty).

samuelklee · 2017-09-26T18:21:11Z

...g/broadinstitute/hellbender/tools/copynumber/utils/segmentation/KernelSegmenterUnitTest.java

+ * @author Samuel Lee &lt;slee@broadinstitute.org&gt;
+ */
+public final class KernelSegmenterUnitTest extends BaseTest {
+    private static final int RANDOM_SEED = 1;   //reset seed before each simulated test case


This was meant to be an imperative to future developers...I'll make it more explicit (//make sure to reset random seed to this value before each simulated test case)

samuelklee · 2017-09-26T18:36:05Z

...g/broadinstitute/hellbender/tools/copynumber/utils/segmentation/KernelSegmenterUnitTest.java

+public final class KernelSegmenterUnitTest extends BaseTest {
+    private static final int RANDOM_SEED = 1;   //reset seed before each simulated test case
+
+    @DataProvider(name = "dataKernelSegmenter")


Good points. Added everything except the sinusoidal background case. Although useful, I think it's a bit beyond the scope of the unit test.

samuelklee · 2017-09-26T18:36:45Z

.../java/org/broadinstitute/hellbender/tools/copynumber/utils/segmentation/KernelSegmenter.java

+        for (final int tauPrime : indices) {
+            D += kernelApproximationDiagonal[tauPrime];
+            final double ZdotW = calculateZdotWAndModifyW(reducedObservationMatrix, p, W, tauPrime, 1);
+            V += 2. * ZdotW + kernelApproximationDiagonal[tauPrime];


I like making my doubles explicit...just a (bad?) habit!

samuelklee · 2017-09-26T18:37:23Z

.../java/org/broadinstitute/hellbender/tools/copynumber/utils/segmentation/KernelSegmenter.java

+            changepointCandidates.addAll(windowCostLocalMinima.subList(0, Math.min(maxNumChangepoints, windowCostLocalMinima.size())));
+        }
+
+        if (changepointCandidates.isEmpty()) {


Decided to add a warning and return an empty list in the case of no data.

samuelklee · 2017-09-26T18:45:20Z

.../java/org/broadinstitute/hellbender/tools/copynumber/utils/segmentation/KernelSegmenter.java

+
+    private final List<T> data;
+
+    public KernelSegmenter(final List<T> data) {


I think I was allowing for the possibility of passing the data once and finding multiple sets of changepoints. This could also be done with a static method, I guess...not sure if there are any advantages/disadvantages either way, so I'll leave it as is, if you don't mind!

mbabadi · 2017-09-26T19:11:43Z

I am satisfied :)

samuelklee · 2017-09-26T19:23:00Z

Thanks! Fixed up some inadvertent missed changes. Will squash and merge once tests pass, if @davidbenjamin gives the go ahead!

davidbenjamin · 2017-09-26T19:55:11Z

@samuelklee I sure do.

samuelklee force-pushed the sl_kernel_segmenter branch from 2ba7ab7 to 5737a46 Compare September 19, 2017 17:47

Added KernelSegmenter.

9de9ecc

samuelklee force-pushed the sl_kernel_segmenter branch from 5737a46 to 9de9ecc Compare September 19, 2017 17:49

samuelklee requested review from mbabadi and davidbenjamin September 19, 2017 17:52

davidbenjamin reviewed Sep 19, 2017

View reviewed changes

Addressed comments and fixed tests.

f9246d4

Rewrote reduced-observation matrix calculation as a matrix multiplica…

e9f4456

…tion.

mbabadi requested changes Sep 26, 2017

View reviewed changes

Addressed second round of comments.

6eae92f

samuelklee commented Sep 26, 2017

View reviewed changes

samuelklee added 3 commits September 26, 2017 15:00

Forgot to revert a refactoring.

7c0f3f0

Forgot some T -> DATA.

09a8114

Apparently I forgot everything.

51d9d6c

mbabadi approved these changes Sep 26, 2017

View reviewed changes

More missed changes.

7f7d86e

samuelklee merged commit eba2346 into master Sep 26, 2017

samuelklee deleted the sl_kernel_segmenter branch September 26, 2017 20:41

This was referenced Oct 10, 2017

Multi-sample CNV segmentation #2924

Closed

Simpler and faster allele fraction likelihoods for HMM segmentation #2937

Closed


		private final List<T> data;

		public KernelSegmenter(final List<T> data) {

Added KernelSegmenter. #3590

Added KernelSegmenter. #3590

Conversation

samuelklee commented Sep 19, 2017

samuelklee commented Sep 19, 2017

davidbenjamin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samuelklee commented Sep 20, 2017

codecov-io commented Sep 20, 2017 • edited Loading

Codecov Report

davidbenjamin commented Sep 20, 2017

samuelklee commented Sep 26, 2017

mbabadi commented Sep 26, 2017 via email

mbabadi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mbabadi commented Sep 26, 2017

samuelklee commented Sep 26, 2017

davidbenjamin commented Sep 26, 2017

codecov-io commented Sep 20, 2017 •

edited

Loading