devitocodes / devito

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

compiler: Fix ConditionalDimensions on SubDomains #2357

Closed EdCaunt closed 2 months ago

EdCaunt commented 2 months ago

Tweak dimension extraction from conditions so that ConditionalDimensions can have SubDimension parents.

Now you can do

class Middle(SubDomain):
    name = 'middle'
    def define(self, dimensions):
        return {x: x, y: ('middle', 2, 4)}
mid = Middle() 
my_grid = Grid(shape = shape, subdomains = (mid, ))

_, yi = mid.dimensions
condition = Lt(sdf, 1).subs(y, yi)
ci = ConditionalDimension(name='ci', condition=condition, parent=yi)

and the ConditionalDimension will be nested inside the SubDimension in the generated loops.

mloubout commented 2 months ago

So you still needs the subs eventhough the parent is yi ?

condition = Lt(sdf.subs(y, yi), 1) would maybe make more sense.

Is there a test/example for it?

codecov[bot] commented 2 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 79.44%. Comparing base (42cce7e) to head (5fac196). Report is 5 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #2357 +/- ## ======================================= Coverage 79.44% 79.44% ======================================= Files 232 232 Lines 43607 43607 Branches 8072 8072 ======================================= Hits 34643 34643 Misses 8208 8208 Partials 756 756 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

EdCaunt commented 2 months ago

So you still needs the subs eventhough the parent is yi ?

condition = Lt(sdf.subs(y, yi), 1) would maybe make more sense.

Is there a test/example for it?

Yeah, you still need the .subs. A test needs to be added, it would probably also be a good idea to add an example

EdCaunt commented 2 months ago

Closed as replicates a fix already in #2050