Fix parallel increments #1446
Fix parallel increments #1446
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1446 +/- ##
==========================================
- Coverage 87.13% 85.93% -1.21%
==========================================
Files 189 189
Lines 27437 27493 +56
Branches 3717 3722 +5
==========================================
- Hits 23908 23625 -283
- Misses 3104 3439 +335
- Partials 425 429 +4
Continue to review full report at Codecov.
|
FabioLuporini
force-pushed
the
fix-parallel-incs
branch
2 times, most recently
from
cd2fa96 to
d243b7d
Compare
devito/passes/iet/openmp.py
Outdated
| for i in reduction: | ||
| if i.is_Indexed: | ||
| # OpenMP expects a size not an index as input of reduction, | ||
| # such as reduction(+:f[d_m:d_M+1]) |
There was a problem hiding this comment.
Need to update this since not the generated anymore.
There was a problem hiding this comment.
gotcha, fixed
tests/test_gpu_openmp.py
Outdated
| @@ -194,7 +194,7 @@ def test_timeparallel_reduction(self): | |||
| assert not tree.root.pragmas | |||
| assert len(tree[1].pragmas) == 1 | |||
| assert tree[1].pragmas[0].value ==\ | |||
| 'omp target teams distribute parallel for collapse(3) reduction(+:f[0])' | |||
| 'omp target teams distribute parallel for collapse(3) reduction(+:f[0:1])' | |||
There was a problem hiding this comment.
This should have been caught by if i.is_Indexed: and still be f[0] since it's a single index. Is it from my PR?
There was a problem hiding this comment.
no, from mine, I was just handling both cases (number and symbol) homegenously. But I've now rolled back to previous behaviour
tests/test_dle.py
Outdated
| @@ -523,6 +521,62 @@ def test_scheduling(self): | |||
| assert not iterations[3].is_Affine | |||
| assert 'schedule(dynamic,chunk_size)' in iterations[3].pragmas[0].value | |||
|
|
|||
| @pytest.mark.parametrize('so', [0, 1, 2]) | |||
| @pytest.mark.parametrize('dim', [1, 2]) | |||
There was a problem hiding this comment.
Is the dim=0 case fix with the other part of that PR (the reduction isn't picked up if most outer loop).
There was a problem hiding this comment.
I'm gonna check now but I think so since the assert 'reduction(...) in op` is now for all test cases
There was a problem hiding this comment.
yes, it looks correct:
#pragma omp parallel num_threads(nthreads)
{
#pragma omp for collapse(3) schedule(static,1) reduction(+:f[0:f_vec->size[0]])
for (int x = x_m; x <= x_M; x += 1)
{
for (int y = y_m; y <= y_M; y += 1)
{
for (int z = z_m; z <= z_M; z += 1)
{
f[y] += u[t0][x + 1][y + 1][z + 1] + 1;
}
}
}
}
There was a problem hiding this comment.
That's still the y dimension not x, need to change @pytest.mark.parametrize('dim', [0, 1, 2]) to get that previously problematic case.
There was a problem hiding this comment.
Added 0 back to dims and seems to be working fine now.
mloubout
force-pushed
the
fix-parallel-incs
branch
from
3df12e5 to
3cd1b2c
Compare
reductionclauses andpragma omp atomic(now avoided if unnecessary)