Fix AttributeError with None obs shape and tf

[Fabien-Couthouis]

Why are these changes needed?

When obs shape is None, calling v.value lead to AttributeError: 'NoneType' object has no attribute 'value' in ModelV2 with tensorflow. This fix adds a check for None value:

batch_dims = [
                v if isinstance(v, (int, type(None))) else v.value
                for v in obs.shape[:-1]
            ]

Related issue number

Closes #16286

Checks

• I've run scripts/format.sh to lint the changes in this PR.
• I've included any doc changes needed for https://docs.ray.io/en/master/.
• I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
• Testing Strategy
  • Unit tests
  • Release tests
  • This PR is not tested :(

[Fabien-Couthouis]

@GoingMyWay

[GoingMyWay]

@GoingMyWay

@Fabien-Couthouis Hi, I guess the same issue may happen for torch. I ran PyTorch variants of A3C and PPO and I got RuntimeError: input.size(-1) must be equal to input_size.. I am debugging the code. Hope I can fix this issue.

[GoingMyWay]

@Fabien-Couthouis. I did not find the bug. After downgrading the ray from 1.10.0 to 1.4.1, the bug disappeared. 🤣

[Fabien-Couthouis]

@GoingMyWay I am not sure the your issue is related to this one, you can open a new issue maybe

[gjoliver]

then you don't need this line anymore. please delete?

[Fabien-Couthouis]

In fact, v.value can still be None:

import tensorflow.compat.v1 as tf
tf.disable_v2_behavior()

obs = tf.placeholder(tf.float32, shape=[None, None, 128, 3], name="X")
print(obs.shape) 
>>> TensorShape([Dimension(None), Dimension(None), Dimension(128), Dimension(3)])

def get_value(v):
    if v is None:
        return -1
    elif isinstance(v, int):
        return v
    else:
        return v.value 

batch_dims = [get_value(v) for v in obs.shape[:-1]]
print(batch_dims)
>>> [None, None, 128]
batch_dims = [-1 if v is None else v for v in batch_dims]
print(batch_dims)
>>> [-1, -1, 128]

But still, this logic can be moved in get_value: bd0e751

[gjoliver]

good point. yeah, it probably should be

if v is None or v.value is None:
   return -1

?

[Fabien-Couthouis]

Checking v.value is None before isinstance(v, int) will lead to something like "ValueError, int type does not have a .value method" if v is an integer. Can change:

def get_value(v):
  if v is None:
      return -1
  elif isinstance(v, int):
      return v
  elif v.value is None:
      return -1
  else:
      return v.value

to:

def get_value(v):
  if isinstance(v, int):
      return v
  elif  v is None or v.value is None:
      return -1
  else:
      return v.value

But in general, I prefere to avoid unnecessary calls to .isinstance because this function can be quite slow.

[gjoliver]

@sven1977, can you help merge? thanks.