Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix thresholded_relu to support list datatype #16277

Merged
merged 3 commits into from
Mar 28, 2022
Merged

fix thresholded_relu to support list datatype #16277

merged 3 commits into from
Mar 28, 2022

Conversation

old-school-kid
Copy link
Contributor

added support for list datatype in thresholded_relu similar to relu.

Notebook with the issue and solution.

Auto closes #16273

added support for list datatype
@old-school-kid
Copy link
Contributor Author

Should I added a test for this too?

@gbaned gbaned requested a review from fchollet March 22, 2022 05:38
@google-ml-butler google-ml-butler bot added the keras-team-review-pending Pending review by a Keras team member. label Mar 22, 2022
Copy link
Member

@fchollet fchollet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR!

return inputs * tf.cast(tf.greater(inputs, theta), inputs.dtype)
dtype = getattr(inputs, 'dtype', tf.keras.backend.floatx())
theta = tf.cast(self.theta, dtype)
return inputs * tf.cast(tf.greater(inputs, theta), dtype)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cast here is unnecessary

@@ -59,8 +59,9 @@ def __init__(self, theta=1.0, **kwargs):
self.theta = backend.cast_to_floatx(theta)

def call(self, inputs):
theta = tf.cast(self.theta, inputs.dtype)
return inputs * tf.cast(tf.greater(inputs, theta), inputs.dtype)
dtype = getattr(inputs, 'dtype', tf.keras.backend.floatx())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Switch to dtype = self.compute_dtype

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@fchollet fchollet removed the keras-team-review-pending Pending review by a Keras team member. label Mar 22, 2022
`dtype = self.compute_dtype` from `getattr` and removed cast of `theta`
@google-ml-butler google-ml-butler bot added the keras-team-review-pending Pending review by a Keras team member. label Mar 22, 2022
@old-school-kid
Copy link
Contributor Author

A tangent to the problem and probably a learning point for some:
Why are there three ReLU functions (LeakyReLU, ReLU, ThresholdedReLU) when all three can be obtained from ReLU with different parameters? Also with a bit API change to either to PReLU or ReLU the other one can be obtained. So why do we have 4 namespaces?
TIA

@LukeWood LukeWood removed the keras-team-review-pending Pending review by a Keras team member. label Mar 24, 2022
@LukeWood
Copy link
Contributor

Looks like some comments are still unresolved on the PR. unassigning from Keras team for now.

@old-school-kid
Copy link
Contributor Author

old-school-kid commented Mar 24, 2022

The two comments:

  1. To switch to dtype = self.compute_dtype from gettatr
  2. To remove cast of theta

has been done. Would you please point out to what else comments are unresolved @LukeWood ?
TIA

@google-ml-butler google-ml-butler bot added kokoro:force-run ready to pull Ready to be merged into the codebase labels Mar 27, 2022
@google-ml-butler google-ml-butler bot added the keras-team-review-pending Pending review by a Keras team member. label Mar 28, 2022
@gbaned gbaned removed the keras-team-review-pending Pending review by a Keras team member. label Mar 28, 2022
@copybara-service copybara-service bot merged commit b17b6c1 into keras-team:master Mar 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to pull Ready to be merged into the codebase size:XS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ThresholdedReLU crashes when the input is a list
5 participants