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.81% 86.86% +0.04%
==========================================
Files 233 234 +1
Lines 43759 43886 +127
Branches 8076 8102 +26
==========================================
+ Hits 37990 38122 +132
+ Misses 5061 5059 -2
+ Partials 708 705 -3 ☔ 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)
There was a problem hiding this comment.
will do in my next PR
| __all__ = ['DistReduce'] | ||
|
|
||
|
|
||
| class DistReduce(sympy.Function, Reconstructable): |
There was a problem hiding this comment.
why do you need sympy.Function ?
There was a problem hiding this comment.
well it needs to be a sympy object and the most logical one is a Function in my opinion
| __rkwargs__ = ('op', 'grid', 'ispace') | ||
|
|
||
| def __new__(cls, var, op=None, grid=None, ispace=None, **kwargs): | ||
| obj = sympy.Function.__new__(cls, var, **kwargs) |
There was a problem hiding this comment.
no op is a devito internal type
There was a problem hiding this comment.
(yes, we should start adding types...)
| mapper = { | ||
| OpInc: 'MPI_SUM', | ||
| OpMax: 'MPI_MAX', | ||
| OpMin: 'MPI_MIN', |
There was a problem hiding this comment.
coming from ... ? we don't support reductions that aren't inc/max/min IIRC
devito/types/sparse.py
Outdated
| "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
| @@ -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. | |||
There was a problem hiding this comment.
will fix in my next PR
| def _rcompile_wrapper(cls, **kwargs0): | ||
| def wrapper(expressions, **kwargs1): | ||
| return rcompile(expressions, {**kwargs0, **kwargs1}) | ||
| def _rcompile_wrapper(cls, **kwargs): |
There was a problem hiding this comment.
This kwargs0 was recently changed as well right, now back to it, ok
No description provided.