Runtime checks for declare_same_as

[albertz]

A bit more meta: With all our logic for dim tags, which should actually make it easier to avoid any reshape problems or other shaping problems, why do we still frequently run into such things? The answer is probably too much unnecessary complexity and thus bugs in some parts. But which parts really? What can we remove from it? How can we improve this situation?

Related: rwth-i6/returnn#975

To answer this question: The problem here was that we actually did manual dim math. In LearnedRelativePositionalEncoding, we had:

    out_spatial_dim = spatial_dim - 1 + spatial_dim

...
      remaining_dim = spatial_dim - self.clipping
...
      cond.true, out_spatial_dim_ = nn.concat(
        (left, remaining_dim),
        (self.pos_emb, self.clipped_spatial_dim),
        (right, remaining_dim))
      out_spatial_dim_.declare_same_as(out_spatial_dim)

And:

    self.clipped_spatial_dim = nn.SpatialDim(
      f"{nn.NameCtx.current_ctx().get_abs_name()}:learned-rel-pos",
      dimension=2 * clipping + 1)

I.e.:

out_spatial_dim_
  == spatial_dim - clipping + 2 * clipping + 1 + spatial_dim - clipping
  == 2 * spatial_dim + 1
  != 2 * spatial_dim - 1
  == out_spatial_dim

But the declare_same_as anyway just overwrote this.

Maybe in this case, we could have detected this statically. But in the general case, there are always cases where we can not detect this at compilation time, and only at runtime.

At some point, we planned to actually add such runtime checks for declare_same_as. Maybe this is a reminder. This would have allowed to much more easily detect this problem.

Originally posted by @albertz in rwth-i6/returnn_common#238 (comment)

[albertz]

Note there is this warning, which is actually not triggered here (I think):

          # Note: Instead of making this a warning, we could also enforce this at some point.
          #   The user should be able to fix `extern_data` in the config such that this is correct in the first place.
          #   Also, in addition to this warning, we might want to add some runtime check on the eq of the dyn sizes.
          print(
            "Warning: assuming dim tags are same with different size placeholders: %r vs %r" % (
              self.dyn_size, other_same_base.dyn_size))

Related: #1143, #1141

[albertz]

Note there is Data.get_placeholder_with_runtime_sanity_checks but this is only used via the optional config option debug_runtime_sanity_checks.

[albertz]

One thing we should do here: For this example case:

out_spatial_dim_
  == spatial_dim - clipping + 2 * clipping + 1 + spatial_dim - clipping
  == 2 * spatial_dim + 1
  != 2 * spatial_dim - 1
  == out_spatial_dim

In out_spatial_dim_.declare_same_as(out_spatial_dim), we should make sure we really get to this warning mentioned above (or even exception with #1143, #1141). This should be a test case.

More generally: declare_same_as is fine as long as some dim is undefined. But when it is supposed to combine two seemingly different dims, then it hits this case.

Instead of #1143, #1141, maybe we should make a runtime check here?

[albertz]

In #975, there is the discussion to completely remove declare_same_as, as basically all its use cases can be replaced by more explicit alternatives. Also runtime checks (comparing two different dim tags) could be done more explicitly, if the user wishes to add them.