devito icon indicating copy to clipboard operation
devito copied to clipboard

dsl: Overhaul custom fd weights

Open rhodrin opened this issue 3 years ago • 5 comments

Work in progress. TBC.

Aside from the actual implementation, we'll also need to discuss legacy support at some stage.

rhodrin avatar Mar 24 '21 21:03 rhodrin

Codecov Report

Merging #1644 (e428923) into master (cde7a8e) will decrease coverage by 18.60%. The diff coverage is 59.09%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master    #1644       +/-   ##
===========================================
- Coverage   88.88%   70.28%   -18.61%     
===========================================
  Files         210      162       -48     
  Lines       33634    21267    -12367     
  Branches     4391     3493      -898     
===========================================
- Hits        29896    14947    -14949     
- Misses       3245     5720     +2475     
- Partials      493      600      +107     
Impacted Files Coverage Δ
devito/finite_differences/__init__.py 100.00% <ø> (ø)
devito/finite_differences/tools.py 79.03% <ø> (-11.20%) :arrow_down:
devito/types/dense.py 75.41% <ø> (-17.96%) :arrow_down:
devito/finite_differences/finite_difference.py 72.91% <36.36%> (-23.09%) :arrow_down:
devito/finite_differences/derivative.py 72.02% <80.00%> (-14.40%) :arrow_down:
devito/types/equation.py 85.71% <100.00%> (-12.99%) :arrow_down:
devito/core/autotuning.py 13.18% <0.00%> (-80.00%) :arrow_down:
devito/passes/clusters/buffering.py 20.32% <0.00%> (-75.09%) :arrow_down:
devito/data/utils.py 21.83% <0.00%> (-70.75%) :arrow_down:
devito/builtins/initializers.py 24.26% <0.00%> (-67.65%) :arrow_down:
... and 144 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update cde7a8e...e428923. Read the comment docs.

codecov[bot] avatar Mar 24 '21 22:03 codecov[bot]

@rhodrin

Aside from the actual implementation, we'll also need to discuss legacy support at some stage.

First thought: drop and burn release

Second thought: provide a mini (mini) tutorial explaining how to transition from old to new

FabioLuporini avatar Mar 25 '21 07:03 FabioLuporini

RIght what I had in mind. The it'll be trivial to extend it to allowing (ind, weight) pairs as coeffs for people who really want custom stencils in general (like 3x3 kernel for first derivative)

For legacy, not completely sure yes to be discussed.

mloubout avatar Mar 25 '21 12:03 mloubout

First thought: drop and burn release

Second thought: provide a mini (mini) tutorial explaining how to transition from old to new

Sounds good to me. We could add a new tutorial in the userapi folder which also explains the transformation and we'll also need to update the DRP tutorial of course.

rhodrin avatar Mar 25 '21 13:03 rhodrin

RIght what I had in mind. The it'll be trivial to extend it to allowing (ind, weight) pairs as coeffs for people who really want > custom stencils in general (like 3x3 kernel for first derivative)

For legacy, not completely sure yes to be discussed.

Yeah, indeed. Will implement that functionality. It's just some minor editing here and there.

Re. legacy support, we could for the time being add a decorator to Eq that based on the equation + presence of a coefficients object just reassembles the equation in the new form. It's not pretty, but should be fairly simple to implement and then cut out when we deprecate.

rhodrin avatar Mar 25 '21 14:03 rhodrin