Unexpected aliasing with multiple Grids

[navjotk]

MFE:

from devito import Grid

g1 = Grid(shape=(10, 10))
g2 = Grid(shape=(20, 20))

x1, y1 = g1.dimensions
x2, y2 = g2.dimensions

x1 is x2 # True

What happens: Dimensions created automatically for grids that were explicitly created as two separate objects end up being the same dimension underneath.

What I expect: Automatically created dimensions should be unique

Why: Principle of least surprise

[mloubout]

Does == returns true as well. I seem to remember this would generate two x loop in an operator so they still treated as different but may be wrong

[FabioLuporini]

Disagree, I don't see any reason why those Dimensions should be different objects since they do not carry any data and they represent, both syntactically and semantically, the same symbolic entity

Does == returns true as well

if they're identical (is comparison => same object), definitely == should return True !

Also note this is in line with SymPy:

In [4]: from sympy import Symbol

In [5]: a = Symbol('a')

In [6]: b = Symbol('a')

In [7]: a is b
Out[7]: True

[navjotk]

they represent, both syntactically and semantically, the same symbolic entity

They do not. For all you know g1.dimensions could represent x, y and g2.dimensions could represent x, z. The user has not mentioned any dimensions to us - Devito created dimensions in the background. It is absolutely wrong to assume that they mean the same dimensions.

Also note this is in line with SymPy:

The difference here being that you explicitly provided the same name to two objects. Not the case with the Devito example. Hence the violation of least surprise.

[FabioLuporini]

Devito created dimensions in the background. It is absolutely wrong to assume that they mean the same dimensions.

That's a fair point.

We should update the docstring accordingly, because devito's default names for grid dimensions will be x, y, z

[navjotk]

We should update the docstring accordingly, because devito's default names for grid dimensions will be x, y, z

I disagree. I think the code creating default dimensions should check for the existence of these dimensions and choose a different name if they exist already. Aliasing should only ever be explicit.

[FabioLuporini]

Why would that be an issue anyway? Multi-Grid Operators are (very) loosely supported anyway.

Even if we were to do that (and again, I still disagree we should), it would require a non-negligible effort, unless (for example) we want to reach the end of the test suite having allocated thousands of symbols only for the SpaceDimensions. Not to talk about the quality of the generated code, which would be dependant on the ordering of creation of objects.. that would really be ugly

if the user wants different names, they have an escape hatch for that

if we want to support Multi-Grid Operators properly, then there's a non-negligible amount of work that needs to be done. Higher order objects from which creating "suitably connected" Grids (with unique Dimension names for example) is one possible example

[navjotk]

we want to reach the end of the test suite having allocated thousands of symbols only for the SpaceDimensions.

What is wrong with allocating thousands of symbols when running thousands of tests, as a matter of principle?

Not to talk about the quality of the generated code, which would be dependant on the ordering of creation of objects.. that would really be ugly

Only if you're using multiple grids with their default dimensions.

if the user wants different names, they have an escape hatch for that

Same argument made in reverse: If someone wants multiple dimensions to map to the same thing, they have a way to do it. However this is not a sensible default.

if we want to support Multi-Grid Operators properly, then there's a non-negligible amount of work that needs to be done. Higher order objects from which creating "suitably connected" Grids (with unique Dimension names for example) is one possible example

Sure, more steps are required to get to full Multigrid support. How is that relevant here? This issue is about sensible defaults. Aliasing automatically created objects is not a sensible default.

@ggorman likes to quote the "Principle of Least surprise" the most. Let's hear what he thinks the principle means in this case.

[mloubout]

Devito created dimensions in the background. It is absolutely wrong to assume that they mean the same dimensions.

This is actually mathematically sane to use the same dimensions. If the user does not provide its own dimension, what the grid creation actually means is "Give me a discrete representation of the cartesian 1/2/3d space starting at origin and ending at origin + extent". But the cartesian axis are fixed and default and should be so. This is the same assumption every single tensor/array package has. Everything leaves on the same basis unless specified otherwise.

[navjotk]

This is the same assumption every single tensor/array package has.

Can you cite a specific example of this?

and should be so.

There is very little reason to create multiple grids that have the same dimensions.

Everything leaves on the same basis

Yes - same basis mathematically, and same Dimension object in Devito are not the same though. e.g. DerivedDimensions live on the same basis but are not the same object syntactically or semantically in Devito.