api: Fix custom coefficients inlining #2524
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2524 +/- ##
==========================================
- Coverage 87.32% 87.31% -0.01%
==========================================
Files 238 238
Lines 45829 45847 +18
Branches 4060 4060
==========================================
+ Hits 40020 40033 +13
- Misses 5125 5129 +4
- Partials 684 685 +1 ☔ View full report in Codecov by Sentry. |
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
| @@ -98,7 +98,7 @@ def _apply_coeffs(cls, expr, coefficients): | |||
| if not mapper: | |||
| return expr | |||
|
|
|||
| return expr.xreplace(mapper) | |||
| return expr.subs(mapper) | |||
There was a problem hiding this comment.
@FabioLuporini this was the issue. If you had something like Eq(u, u.dx2 + 2 * u.laplace, substitutions=...) then xreplace only replaces the firs appearance so it was using custom coeffs for some and standard coeffs for other. Code looks much better (less. weights and derivatives) with the fix.
| @@ -83,7 +83,7 @@ | |||
| "name": "stdout", | |||
| "output_type": "stream", | |||
| "text": [ | |||
| "Eq(-u(t, x, y)/dt + u(t + dt, x, y)/dt + (0.1*u(t, x, y) - 0.6*u(t, x - h_x, y) + 0.6*u(t, x + h_x, y))/h_x, 0)\n" | |||
| "Eq(-u(t, x, y)/dt + u(t + dt, x, y)/dt + 0.1*u(t, x, y)/h_x - 0.6*u(t, x - h_x, y)/h_x + 0.6*u(t, x + h_x, y)/h_x, 0)\n" | |||
There was a problem hiding this comment.
so we lose the flop reduction here?
There was a problem hiding this comment.
This is only the symbolic equation, not the operator.
this is only an issue for the legacy custom coefficients API