-
Notifications
You must be signed in to change notification settings - Fork 27.7k
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
Remove a TF usage warning and rework the documentation #9756
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not a big fan of removing the warning, but if there is no alternative, using the documentation to say the same thing is good enough.
Not in favor of this to be honest...Warnings / Print statements are super important IMO. It's weird that |
XLA/Graph/TFlite execution are not made to have things to be printed, this includes some of the assert usage as well. |
@patrickvonplaten TF XLA has few other known issues https://www.tensorflow.org/xla/known_issues |
Thanks for sharing! I'm still very surprised by your message:
To me, this means that every TF Repo that wants to be executable with XLA has no |
I agree, it is, but I see very rarely |
Can we use
intead? |
@sgugger yes, using the internal TF logger should work as expected, but I'm not sure if it will bring any conflict with the actual transformers logger in terms of configuration. Do you want me to use it instead, and we will see later if there is indeed any conflict? |
Yeah I think it would be best to have a warning the user can't silence with our centralized logging that none. |
Ok done! I restored the warning but with the internal TF logger. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works for me, thanks for taking care of it.
What does this PR do?
Recently we moved the warning for boolean argument misused in TF models from
warnings.warning(...)
totf.Print
to avoid the overhelm of messages in the output. Nevertheless, the usage oftf.Print
prevent all our TF models to be compiled and executed with XLA and quantized with TFLite because thePrint
operator is not supported in XLA and TFLite.As a solution for both issues (logging overhelm + XLA compilation/execution) I propose to simply remove the logs and state the use case directly inside the documentation for all the TF models.