Problem in dependency analysis

[havogt]

Adding an ij-cache for the rxm or rxp placeholder in the following example will give an out-of-bounds in the cache storage
https://github.com/MeteoSwiss-APN/cosmo-prerelease-gridtools/blob/2dd9994914714326e2f4cf0fb6f5d4d8aab9ef0a/dycore_gridtools/src/dycore/Stencils/HorizontalDiffusion/HorizontalDiffusionType2Limiter.cpp#L140.

@cosunae and I debugged the issue and we found the following problem:

• The size of the cache is deduced to be <-2,1,-2,1> which is correct.
• Dependency analysis computes the wrong extent<-2,1,-2,1> (the region of computation) for the LimitFluxStage.
• However this stage has access extent<0, 1, 0, 1> on the rxp, rxm accessors. I.e. it will access at i+2 and j+2 which is out-of-bounds.

From our analysis we conclude the LimitFluxStage should have extent<-1,0,-1,0> (which is quite obvious as it is the second last stage and therefore we can directly read the extents from the accessors).

All other extents of the stages are correct. A possible problem could be the pattern in->flx->rxp->flx->out, i.e. we are writing and reading twice into/from the same temporary flx.

Workaround: The problem can be fixed by using make_stage_with_extent.

I attach the file which contains at the bottom the result of the dependency analysis. The first type contains the extents for the stages. The second type computed extended extents for the placeholders.

HorizontalDiffusionType2Limiter.txt

[mbianco]

Could you put the link to the Laplacian operator?

[mbianco]

The limitflux stage should have a <-1,1,-1,1> right?

[havogt]

The Laplacian is here: https://github.com/MeteoSwiss-APN/cosmo-prerelease-gridtools/blob/2dd9994914714326e2f4cf0fb6f5d4d8aab9ef0a/dycore_gridtools/src/dycore/Stencils/HorizontalDiffusion/HorizontalDiffusionFunctions.hpp#L40

[havogt]

I think the limitflux stage should be <-1,0,-1,0> as these are the accesses on the data stage which is the consumer.

[mbianco]

Sorry,I meant the extent computed by the analysis...

[havogt]

Yes, I think so.

[havogt]

And I think the dependency analysis has this correct if I read the type correctly.

My understanding of the following is, that the the data stage is computed on <0, 0, 0, 0, 0, 0>, the flux stage on <-2, 1, -1, 0, 0, 0>, etc. Correct?

  boost::mpl::v_item<gridtools::extent<0, 0, 0, 0, 0, 0>,
                    boost::mpl::v_item<gridtools::extent<-2, 1, -1, 0, 0, 0>,
                                       boost::mpl::v_item<gridtools::extent<-1, 1, -1, 1, 0, 0>,
                                           boost::mpl::v_item<gridtools::extent<-2, 1, -2, 1, 0, 0>,
                                                              boost::mpl::v_item<gridtools::extent<-2, 2, -2, 2, 0, 0>,

[havogt]

Mauro proposed to split the fluxstage into 2, one for each flux.

It doesn't change the result: both flux stages have the same wrong extent <-2,1,-2,1,0,0>.

[havogt]

Standalone example #922

[havogt]

Just for reference: actually this is not a problem with the cache. If you don't specify a halo for the datastore then you will get an out of bounds as well. The reason why you don't get it with halo is the over-allocation of the temporaries. In that case we just read from the padding area.

[havogt]

The dependency analysis requires that each field must be written at most once. For the release we should check that this is very visible in the documentation and think about protections.

[mbianco]

I think this check should be done outside of the extent analysis. Doing it in there would require to add an additional flag to the placeholders to check if they have been found to be written in a previous iteration of the algorithm, but this additional flag would be used only for checking the rule. I would check the rule in an independent metafunction that just checks that, and it should not be difficult to implement (we just need to check that in a composition a certain placeholder is indicated as output only once). Now, do it with the meta would be a natural solution, I'm not sure how long it would take for me to do it. Opinions?