compiler: Support for C-level MPI_Allreduce #2344
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2344 +/- ##
==========================================
- Coverage 86.66% 79.44% -7.23%
==========================================
Files 232 233 +1
Lines 43445 43698 +253
Branches 8063 8098 +35
==========================================
- Hits 37653 34714 -2939
- Misses 5087 8228 +3141
- Partials 705 756 +51 ☔ View full report in Codecov by Sentry. |
| [dv.Inc(s, p), dv.Eq(mr.n[0], s)], | ||
| name='sum') | ||
| op.apply(**kwargs) | ||
| op = dv.Operator([dv.Eq(s, 0.0)] + eqns + |
There was a problem hiding this comment.
nitpicking: can probalby wrap (since everywhere in this file) the s=0, eqs, s+=expr, n[0]=s into an make_reduction(eqns, expr)
| __all__ = ['DistReduce'] | ||
|
|
||
|
|
||
| class DistReduce(sympy.Function, Reconstructable): |
There was a problem hiding this comment.
why do you need sympy.Function ?
| __rkwargs__ = ('op', 'grid', 'ispace') | ||
|
|
||
| def __new__(cls, var, op=None, grid=None, ispace=None, **kwargs): | ||
| obj = sympy.Function.__new__(cls, var, **kwargs) |
| mapper = { | ||
| OpInc: 'MPI_SUM', | ||
| OpMax: 'MPI_MAX', | ||
| OpMin: 'MPI_MIN', |
| "or iterable (e.g., list, np.ndarray)" % key) | ||
| if d in key.dimensions and not self.alias: | ||
| # From a reconstruction which leaves `dimensions` intact | ||
| return key |
There was a problem hiding this comment.
For future reference, this is very problematic (both if and else case) because now key's parent does not match the sparsefunction that uses it as a subfunction so can lead to very problematic runtime args/post_process so we may need to completely avoid this rebuild and always create a new subfunc using the key's data (need a '_local' initalizer
| i = dv.Dimension(name='mri',) | ||
| n = dv.Function(name='n', shape=(1,), dimensions=(i,), grid=grid, | ||
| dtype=dtype, space='host') | ||
| n.data[:] = 0 |
There was a problem hiding this comment.
Shouldn't this initialise to zero already? Also do you need to set the space order of n to 0 so that it doesn't assign a halo?
| @@ -453,7 +453,7 @@ def test_sum_sparse(self): | |||
|
|
|||
| def test_min_max_sparse(self): | |||
| """ | |||
| Test that mmin/mmax work on SparseFunction | |||
| Test that mmin/mmax work on SparseFunction. | |||
No description provided.