devito
devito copied to clipboard
`SubDimension` 'thickness' inconsitently defined
Take a look at:
@classmethod
def left(cls, name, parent, thickness, local=True):
lst, rst = cls._symbolic_thickness(name)
return cls(name, parent,
left=parent.symbolic_min,
right=parent.symbolic_min+lst-1,
thickness=((lst, thickness), (rst, 0)),
local=local)
@classmethod
def right(cls, name, parent, thickness, local=True):
lst, rst = cls._symbolic_thickness(name)
return cls(name, parent,
left=parent.symbolic_max-rst+1,
right=parent.symbolic_max,
thickness=((lst, 0), (rst, thickness)),
local=local)
@classmethod
def middle(cls, name, parent, thickness_left, thickness_right, local=False):
lst, rst = cls._symbolic_thickness(name)
return cls(name, parent,
left=parent.symbolic_min+lst,
right=parent.symbolic_max-rst,
thickness=((lst, thickness_left), (rst, thickness_right)),
local=local)
The 'thickness' properties mean something different for middle vs left/right - this should probably be made consistent or the actual properties changed?
Note that this inconsistency results in 'SubDomain'.shape
being computer incorrectly. A 'fix' is present in https://github.com/devitocodes/devito/tree/fix_subdim_size2 but I don't really like this and the underlying issue should probably be addressed now. I did start producing a 'proper' fix but it probably results in user code changing so wanted to discuss it here first.
Maybe we do not need to change the user code but only how the input is processed to create the subdomain:
- Turn the thickness left/right into a dim_width
- Remove thickness internally
- Size is just dim_width
This would prbably mess up lot of sthings so maybe not as trivial as I picture it.
I'm looping in @tjb900 because it's a relevant discussion for him
I'll take a proper look tmr
Note: Temporary fix in #1186.
@rhodrin reminder that in your next PR you may wanna expand that comment in _arg_defaults with a reference to this issue