Skip to content

Commit

Permalink
Fixed low level bug in ConcurrentyOps where it was not clearing the w…
Browse files Browse the repository at this point in the history
…orkspace in all situations,

resulting in really weird bugs in downstream customers of EJML
  • Loading branch information
lessthanoptimal committed Sep 24, 2023
1 parent 03d1c29 commit bc7db95
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 3 deletions.
9 changes: 7 additions & 2 deletions main/ejml-core/src/pabeles/concurrency/ConcurrencyOps.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2021, Peter Abeles. All Rights Reserved.
* Copyright (c) 2023, Peter Abeles. All Rights Reserved.
*
* This file is part of Efficient Java Matrix Library (EJML).
*
Expand Down Expand Up @@ -195,6 +195,9 @@ public static <T> void loopBlocks( int start, int endExclusive, GrowArray<T> wor
final ForkJoinPool pool = ConcurrencyOps.pool;
int numThreads = pool.getParallelism();

// Make sure there are no stale results, even if nothing is run
workspace.reset();

int range = endExclusive - start;
if (range == 0) // nothing to do here!
return;
Expand Down Expand Up @@ -222,6 +225,9 @@ public static <T> void loopBlocks( int start, int endExclusive, int minBlock,
final ForkJoinPool pool = ConcurrencyOps.pool;
int numThreads = pool.getParallelism();

// Make sure there are no stale results, even if nothing is run
workspace.reset();

int range = endExclusive - start;
if (range == 0) // nothing to do here!
return;
Expand All @@ -235,7 +241,6 @@ public static <T> void loopBlocks( int start, int endExclusive, int minBlock,

private static <T> void runLoopBlocks( int start, int endExclusive, GrowArray<T> workspace,
IntRangeObjectConsumer<T> consumer, ForkJoinPool pool, int blockSize ) {
workspace.reset();
try {
pool.submit(new IntRangeObjectTask<>(start, endExclusive, blockSize, workspace, consumer)).get();
} catch (InterruptedException | ExecutionException e) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2021, Peter Abeles. All Rights Reserved.
* Copyright (c) 2023, Peter Abeles. All Rights Reserved.
*
* This file is part of Efficient Java Matrix Library (EJML).
*
Expand Down Expand Up @@ -50,8 +50,12 @@ class TestImplMultiplication_MT_DSCC extends EjmlStandardJUnit {
mult_s_s(5, 10, 5);
mult_s_s(5, 5, 10);
}

// See comment in mult_s_s. This triggered a bug
mult_s_s(10, 10, 0);
}


private void mult_s_s( int rowsA, int colsA, int colsB ) {
int nz_a = RandomMatrices_DSCC.nonzero(rowsA, colsA, 0.05, 0.7, rand);
int nz_b = RandomMatrices_DSCC.nonzero(colsA, colsB, 0.05, 0.7, rand);
Expand All @@ -62,6 +66,10 @@ private void mult_s_s( int rowsA, int colsA, int colsB ) {
DMatrixSparseCSC expected = RandomMatrices_DSCC.rectangle(rowsA, colsB, nz_c, -1, 1, rand);
DMatrixSparseCSC found = expected.copy();

// Make sure the work space is cleaned up. There was a bug where if b has zero columns stitching would
// throw an exception if this wasn't empty
workSpaceMT.grow();

ImplMultiplication_DSCC.mult(a, b, expected, null, null);
ImplMultiplication_MT_DSCC.mult(a, b, found, workSpaceMT);
assertTrue(CommonOps_DSCC.checkStructure(found));
Expand Down

0 comments on commit bc7db95

Please sign in to comment.