@Krovatkin wdyt?

Discussion summary: #77882

@Krovatkin wdyt?

how do we determine which ops to write SymIntArrayRef versions of? Is it (a) all ops that take size inputs, (b) only ops that we expect to consume outputs of dynamic ops like nonzero in models? or something else

(b) only ops that we expect to consume outputs of dynamic ops like nonzero in models?

I believe

ok. relatedly- how does it manifest when one is missing? presumably it would show up as a compile error. but it's not immediately clear to me how that works. Who actually calls the 'compute_shape_expand' variant with symints?

ok. relatedly- how does it manifest when one is missing? presumably it would show up as a compile error.

I think it would show up as a runtime error when a dispatcher wouldn't be able to find a dispatch that takes a version with SymInts.

Who actually calls the 'compute_shape_expand' variant with symints?

The dispatcher. When a user passes sym ints either in python or c++, the dispatcher should dispatch to the expand overload that takes symints?

ok. the dispatcher doesn't call compute_shape_* right?

but i think i see your point, that we would specifically implement a .symint version of a nativefunc for lazytensor, and if we did that, then we have to implement this variant of compute_shape.

@miladm I thought we decided to keep -1 as a plain integer? @suo added support for this case.
I think your check should say:
_sizes[idx].is_symbolic() then do

 auto lazySymIntNode = std::dynamic_pointer_cast<torch::lazy::SymbolicIntNode>(symbolicIntNode);
    auto size_node = lazySymIntNode->node_;
    auto static_value = std::dynamic_pointer_cast<torch::lazy::DimensionNode>(size_node)->getStaticValue();

otherwise just do _sizes[idx].data() .

Addressed.

@miladm i was confused by this part, what does it mean to convert to a static value, this is a real int?

@wconstab this is an int value that returns the static upper bound. (base class ref)

@wconstab that's just my unfortunate naming. staticValue == upperBound.

@miladm are you sure this is correct?

If I run the example below, PyTorch produces an error:

>>> import torch
>>> a = torch.rand(3)
>>> a.expand(-1, 3)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
RuntimeError: The expanded size of the tensor (-1) isn't allowed in a leading, non-existing dimension 0

while your shape inference would silently estimate target_size[0] as 0?

Interesting. When I test on PT and PT/XLA, I don't hit your observation meaning the code errors out as expected. Perhaps my code flow doesn't hit the shape inference the way it does for you? Let me know what you ran to observe the inconsistency.

Sound good. Let's land this and open follow up issues as needed. @Krovatkin

I think we need an extra if in here:

} else {
  if (_sizes[idx].data() == -1) {
      // -1 can't be specified for non-existing dimensions
      TORCH_CHECK(idx >= num_new_dimensions);
       target_size[idx] =  padded_self[idx];
  } else {
     target_size[idx] =  _sizes[idx];
  }
}

That's correct.

I think we need to call the .data() method when assigning to target_size[idx]. Let me know if you think differently @Krovatkin.