tensorflow / swift-apis

Swift for TensorFlow Deep Learning Library
Apache License 2.0
794 stars 133 forks source link

Swifty API discovery via '@available' on raw APIs. #483

Open rxwei opened 4 years ago

rxwei commented 4 years ago

For many raw APIs, we already have corresponding idiomatic APIs that implement all of the raw API's functionality, e.g. Tensor.init(_:) for Raw.cast(_:), Tensor.reshaped(to:) for Raw.reshape(_:shape:), etc. These raw APIs are not recommend also because they are not differentiable. However, raw APIs are more discoverable than the idiomatic APIs to users who are familiar with Python TensorFlow, and there are no recommended alternatives when some differentiability error occurs on the raw API.

As such, we should consider adding an @available attribute to raw APIs that have an idiomatic alternative that implements all of their functionality. Using such a raw API will produce a warning.

public extension Raw {
    ...
    @available(*, deprecated, message: "use the 'reshape(_:)' method on 'Tensor' instead")
    func Raw.reshape<T>(_ x: Tensor<T>, shape: Tensor<Int32>) -> Tensor<T>
    ...
}

Compiler warns if Raw.reshape is used:

test.swift:1:1: warning: 'reshape(_:shape:)' is deprecated: use the 'reshaped(to:)' method on 'Tensor' instead
Raw.reshape(x)
^

Note: We could also use a renamed: argument in an @available attribute, but the message would not be as clear to Python users. For instance, "renamed to 'Tensor.reshaped(to:)'" is less descriptive than "use the 'reshaped(to:)' method on 'Tensor' instead".

Shashi456 commented 4 years ago

@rxwei Could we instead have a more user friendly warning which says consider using x.reshape() instead, just my opinion that for beginners, use the reshape method on 'Tensor' instead might be little difficult to understand.

rxwei commented 4 years ago

Could we instead have a more user friendly warning which says consider using x.reshape() instead, just my opinion that for beginners

Library-provided error messages need to present alternative APIs accurately. x.reshape() is not accurate in that x is undefined and the identifier does not show the function's arity.

eaplatanios commented 4 years ago

Is there a way to disable these warning with some other annotation for intentional uses of the Raw API?

rxwei commented 4 years ago

There is not, unfortunately. I’m definitely seeing possible intensional uses in things like lazy tensor tests, but it appears to me that these warnings are only affecting the end user positively and making code reviews easier.

Shashi456 commented 4 years ago

@rxwei when I said x.reshape() I meant is there any way to have error messages with <var_name>.reshape().

rxwei commented 4 years ago

@rxwei when I said x.reshape() I meant is there any way to have error messages with <var_name>.reshape().

There is no way of doing so, unfortunately.

Shashi456 commented 4 years ago

@rxwei one last thing, what file would be appropriate to place these warnings in?

rxwei commented 4 years ago

@rxwei one last thing, what file would be appropriate to place these warnings in?

This should be implemented modularly as part of the raw operator generation script.