devitocodes / devito

DSL and compiler framework for automated finite-differences and stencil computation
http://www.devitoproject.org
MIT License
544 stars 223 forks source link

indexing #2

Closed mloubout closed 7 years ago

mloubout commented 8 years ago

@navjotk the indexes x,y in __prepare_stencil are flipped (y,x) compared to the rest. It will create problem for non square domains

navjotk commented 8 years ago

@mloubout Thanks for pointing that out. The current code does mix up indexing order but this code is a work in progress. You shall see consistent array access in the next version (either (x,y) or (y,x)).

The idea behind changing the order was to have t as the first indexing parameter to enable vectorisation of operations. The order of (x,y) wouldn't make a difference as far as vectorisation is concerned, but of course it needs to be consistent.

mloubout commented 8 years ago

@navjotk Ok good, sorry for being picky For visibility and to make sense, I think it would be better to rename the indexes. I started with x,y because I wrote the equation for 3D but in practice the best order would be (z,x,y) simpkifying to (z.x) in 2D where z is the depth (positive) and x the position at the surface.

mlange05 commented 8 years ago

Ok, as I'm starting to internalize and automate more of the symbolic manipulations this indexing problem and the current default (x, z) is becoming somewhat annoying. Why are we not using (x, y) in 2D and (x, y, z) in 3D, with (t, x, y) and (t, x, y, z) for time-dependent variables? My argument here is that (x, z) is counter-intuitive and devito is really just a generic PDE solving stencil engine, so FWI-specific conventions should live outside of the devito core.

That being said, once we have untangled some of the current Operator/Propagator assumptions we can hopefully make dimension indices user defined and allow overrides, at which point this can be changed. But in the meantime I would like to impose standard conventions throughout to ease further development.

mloubout commented 7 years ago

THis can be closed