keras.Sequential sometimes states misleading reason for failing to construct model

[briango28]

keras.Sequential.build() builds by running a placeholder tensor through each given layer to build a Functional model.

The method explicitly handles two types of exceptions: NotImplementedError and TypeError, of which the latter is intercepted in order to reraise a ValueError in the case of parameter incompatibility.

However, the mechanism to check whether the layer's call() method is compatible with the Sequential API currently assumes any argument without a default value is a positional argument, presenting some unintended side effects:

1. A call() method with default values for all parameters will be reported to have no positional arguments
2. A call() method with a single keyword-only parameter without a default value will not be detected as erroneous.
3. *args or **kwargs parameters will be included as positional arguments, whether or not they are required for the layer to run

While there is no sane way to detect whether a layer with variable arguments is properly defined, the other issues may be avoided by checking the kind field of each parameter along with the existence of a default value.

try:
    x = layer(x)
except NotImplementedError:
    return
except TypeError as e:
    signature = inspect.signature(layer.call)
    positional_args = [
        param
        for param in signature.parameters.values()
        if param.kind <= inspect.Parameter.VAR_POSITIONAL
    ]
    required_positional_args = [
        param for param in positional_args
        if param.default == inspect.Parameter.empty
    ]
    required_keyword_args = [
        param
        for param in signature.parameters.values()
        if param.kind == inspect.Parameter.KEYWORD_ONLY
        and param.default == inspect.Parameter.empty
    ]
    if not positional_args:
        raise ValueError(
            "Layers added to a Sequential model "
            "should have a single positional argument, "
            f"the input tensor. Layer {layer.__class__.__name__} "
            f"has no positional arguments"
        )
    if len(required_positional_args) > 1:
        raise ValueError(
            "Layers added to a Sequential model "
            "can only have a single positional argument, "
            f"the input tensor. Layer {layer.__class__.__name__} "
            f"has multiple positional arguments: "
            f"{required_positional_args}"
        )
    if required_keyword_args:
        raise ValueError(
            "Layers added to a Sequantial model "
            "can only have a single positional argument, "
            "the input tensor. Layer {layer.__class__.__name__} "
            "has one or more keyword arguments: "
            "{required_keyword_args}"
        )
    raise e

I might've missed some possible cases; feedback would be appreciated.

[fchollet]

Thanks for the report, do you have a code snippet to reproduce the problem as stated in the title?

[briango28]

See the following colab:
https://colab.research.google.com/drive/1p7YnYJpIvpM197VKUX7Ig3kTojpL_ILK?usp=sharing
A TypeError is raised due to incompatible dtypes (tf.string given to a numeric op), but Sequential intercepts it and reraises a misleading ValueError.

The call function signatures are direct implementations of the side effect cases mentioned above.
Note that no. 2 is rather niche because keras prevents a Layer with such a call signature from being instantiated; the example works because the call method is replaced after instantiation.