Misc code generation improvements #2001
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2001 +/- ##
==========================================
- Coverage 87.91% 87.85% -0.07%
==========================================
Files 214 214
Lines 36613 36710 +97
Branches 5538 5550 +12
==========================================
+ Hits 32189 32250 +61
- Misses 3907 3942 +35
- Partials 517 518 +1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
|
|
||
| def __eq__(self, other): | ||
| return (type(self) == type(other) and | ||
| all(i == j for i, j in zip(self.args, other.args))) | ||
| return (type(self) is type(other) and |
| @property | ||
| def symbolic_nbytes(self): | ||
| # TODO: one day we'll have to fix the types/ vs extended_sympy/ thing | ||
| from devito.symbolics import SizeOf # noqa |
There was a problem hiding this comment.
I guess this here is due to circular dependency?
There was a problem hiding this comment.
yes, unavoidable until we perform a systmematic restructuing of types/ and symbolics/
| g = TimeFunction(name='g', grid=grid) | ||
|
|
||
| idx = time + 1 | ||
| s = Indirection(name='ofs0', mapped=idx) |
There was a problem hiding this comment.
not sure I appreciate the need for this indirection class...need to see better
There was a problem hiding this comment.
unused in OSS, but used in PRO
I however added it here , along with a suitable test, because I foresee its use in OSS as well
devito/passes/iet/engine.py
Outdated
|
|
||
| v = i | ||
| def key(i): | ||
| for p, cls in enumerate(priority, 1): |
There was a problem hiding this comment.
Wouldn't it be better with priority as a Dict so that we can easily add new ones with explicit priorities?
There was a problem hiding this comment.
sounds good, changing
devito/passes/iet/engine.py
Outdated
| def _(i, mapper, counter): | ||
| base = 'f' | ||
|
|
||
| kwargs = {k: getattr(i, k, None) for k in i.__rkwargs__} |
There was a problem hiding this comment.
Is there corner cases where an object doesn't have its own rkwargs defined?
There was a problem hiding this comment.
they all inherit from Reconstructable so we should be good
devito/passes/iet/engine.py
Outdated
| kwargs['name'] = '%s%d' % (base, counter[base]) | ||
| kwargs['initializer'] = None | ||
| kwargs['alias'] = True | ||
| v = i._rebuild(**kwargs) |
There was a problem hiding this comment.
Isn't this a bit redundant/overkill? __rkwargs__ will be processed by _rebuild so there is no need to do it here, name/initializer/alias should be the only ones needed
There was a problem hiding this comment.
jeeeeezzz ! True!
devito/passes/iet/engine.py
Outdated
|
|
||
| kwargs = {k: getattr(i, k, None) for k in i.__rkwargs__} | ||
| kwargs['name'] = '%s%d' % (base, counter[base]) | ||
| v = type(i).__base__(**kwargs) |
There was a problem hiding this comment.
my bad, see comment below (TLDR: I'm a stupid)
devito/passes/iet/engine.py
Outdated
| args = [getattr(i, k, None) for k in i.__rargs__] | ||
| args[0] = '%s%d' % (base, counter[base]) | ||
| kwargs = {k: getattr(i, k, None) for k in i.__rkwargs__} | ||
| v = i.func(*args, **kwargs) |
There was a problem hiding this comment.
sorry, this is all leftovers from the old implementation!
That's a really great catch
I'm gonna do this properly now
devito/passes/iet/engine.py
Outdated
|
|
||
| kwargs = {k: getattr(i, k, None) for k in i.__rkwargs__} | ||
| kwargs['name'] = '%s%d' % (base, counter[base]) | ||
| v = i._rebuild(**kwargs) |
There was a problem hiding this comment.
Only name needed not rkwargs
There was a problem hiding this comment.
absolutely yes
| @@ -1624,51 +1627,3 @@ def _arg_values(self, **kwargs): | |||
| raise InvalidArgument("Illegal runtime value for `%s`" % self.name) | |||
| else: | |||
| raise InvalidArgument("TempFunction `%s` lacks override" % self.name) | |||
|
|
|||
|
|
|||
| class AliasFunction(DiscreteFunction): | |||
|
@mloubout should be all sorted now have a look |
|
Merged thanks |
This is mostly about avoiding premature lowering so that we have the necessary info in pro to perform compilation