api: Misc fixes for builtins and harmonic averaging #2520
Conversation
devito/passes/iet/misc.py
Outdated
| @@ -225,6 +226,11 @@ def _(expr): | |||
| return () | |||
|
|
|||
|
|
|||
| @_lower_macro_math.register(SafeInv) | |||
| def _(expr): | |||
| return (('SAFEINV(a, b)', '(((a) < 1e-12 || (b) < 1e-12) ? (0.0F) : (1.0F / (a)))'),) | |||
There was a problem hiding this comment.
the 1e-12 bounds should ideally be pulled from the SafeInv's attributes (default to 1e-12, but allow caller/user to pass a different threshold?)
There was a problem hiding this comment.
it's a safe threshold. But this is also only needed for single precision, double precision should probably default to standard div (not that we ever run double prec)
There was a problem hiding this comment.
yeah, it's just that like that it's a magic number buried deep down inside a compiler pass, so it should rather be pulled from expr itself (can do as a class attribute)
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2520 +/- ##
=======================================
Coverage 87.31% 87.31%
=======================================
Files 238 238
Lines 45771 45810 +39
Branches 4060 4060
=======================================
+ Hits 39965 40000 +35
- Misses 5123 5126 +3
- Partials 683 684 +1 ☔ View full report in Codecov by Sentry. |
| class SafeInv(Differentiable, sympy.core.function.Application): | ||
| _fd_priority = 0 | ||
|
|
||
| def __new__(cls, val, base, **kwargs): |
There was a problem hiding this comment.
is this override just to tolerate potential extra **kwargs (evaluate?) ?
devito/symbolics/inspection.py
Outdated
| @@ -117,7 +117,8 @@ def estimate_cost(exprs, estimate=False): | |||
| 'div': 5, | |||
| 'Abs': 5, | |||
| 'floor': 1, | |||
| 'ceil': 1 | |||
| 'ceil': 1, | |||
| 'SafeInv': 10, | |||
There was a problem hiding this comment.
should you put it futher up, so they remain in descending order?
mloubout
force-pushed
the
wood-run-stable
branch
3 times, most recently
from
03cc79d to
9e8f4df
Compare
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
mloubout
force-pushed
the
wood-run-stable
branch
2 times, most recently
from
d8d60e1 to
25c0e63
Compare
mloubout
force-pushed
the
wood-run-stable
branch
2 times, most recently
from
8c11f8d to
4955b9e
Compare
devito/operator/operator.py
Outdated
| # to avoid out of memory with greedy compilers | ||
| try: | ||
| cfunction = self.cfunction | ||
| except Exception as e: |
There was a problem hiding this comment.
drop try-except; compiler.py's already throws an exception if compilation fails, and that's what the user should see
mloubout
force-pushed
the
wood-run-stable
branch
from
4955b9e to
fb55248
Compare
mloubout
force-pushed
the
wood-run-stable
branch
from
fb55248 to
d437b25
Compare
Fixes #1374