ComponentTensor needs at least an index to have an extent

[ReubenHill]

ComponentTensor makes the below assertion:

class ComponentTensor(Node):
    __slots__ = ('children', 'multiindex', 'shape')
    __back__ = ('multiindex',)

    def __new__(cls, expression, multiindex):
        assert not expression.shape

        # Empty multiindex
        if not multiindex:
            return expression

        # Collect shape
        shape = tuple(index.extent for index in multiindex)
        assert all(s >= 0 for s in shape) ## <- HERE

which cannot be done if none of the indices have an extext.

This typically comes up when creating indices with gem.indices which do not set the extent property of the indices.

[wence-]

ComponentTensor turns free indices into shape. To do this, the indices must have an extent (otherwise what is the resulting shape?)

[ReubenHill]

Perhaps the issue is upside down then and what one really wants is gem.indices to be able to set extent

[wence-]

So ComponentTensor requires that multiindex <= expression.free_indices, so if you do:

i, j = indices()
...
ct = ComponentTensor(expr, (i, j))

What is the situation under which expr has i and j as free indices, but i and j have not had their extents set? I think it is only if you did something like:

expr = Delta(i, j)...

Is that what you're doing?

[ReubenHill]

Ah I see - if I use Indices to index expressions their extent is modified:

indices = gem.indices(len(T.shape))
assert indices[0].extent is None # true
T[indices]
assert indices[0].extent is None # false! Causes assertion error.

I wasn't expecting this.

What is the situation under which expr has i and j as free indices, but i and j have not had their extents set? I think it is only if you did something like:

expr = Delta(i, j)...

Is that what you're doing?

Yes it is.

[wence-]

Ah I see - if I use Indices to index expressions their extent is modified:

indices = gem.indices(len(T.shape))
assert indices[0].extent is None # true
T[indices]
assert indices[0].extent is None # false! Causes assertion error.

I wasn't expecting this.

Ah, OK, Indexed does:

        for index, extent in zip(multiindex, aggregate.shape):
            assert isinstance(index, IndexBase)
            if isinstance(index, Index):
                index.set_extent(extent)

And Index.set_extent says:

    def set_extent(self, value):
        # Set extent, check for consistency
        if self.extent is None:
            self.extent = value
        elif self.extent != value:
            raise ValueError("Inconsistent index extents!")

Is that what you're doing?

Yes it is.

In this case I would do:

i, j, k = (Index(extent=n) for n in some_shape)

Since I assume you have the shape lying around somewhere.

[wence-]

Is this OK to close?

[ReubenHill]

Well Indexed silently changing index.extent feels rather naughty to me. Though I note that the Indexed class and the ComponentTensor class have docstrings. I suppose this is really a sub-issue of #240 .

[wence-]

It's a user-interface "nicety". Indices need to carry their extent, but sometimes it's easier to just make them before you know the extent they'll need and then have it set on use (note that the extent is only ever changed from None to n and subsequently if an index is used we check that the extent and the use matches).

[ReubenHill]

Well Indexed silently changing index.extent feels rather naughty to me. Though I note that the Indexed class and the ComponentTensor class have docstrings. I suppose this is really a sub-issue of #240 .

Apologies this was meant to say that they don't have docstrings

[ReubenHill]

The answer seems to me to be that if you let indices have no extent then it should be very clearly documented somewhere that this will be set upon use