-
Notifications
You must be signed in to change notification settings - Fork 524
Fix special scalar handling for addcdiv and addcmul #3953
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
Conversation
|
Test actually passed on master, so I guess rebase should solve the issue here. |
The CI error appears to be irrelevant. We've seen this failure in our internal tests before but it's not quite reproducible. Any idea what's wrong here?
|
rerun should solve this issue, let me trigger it |
@@ -741,6 +741,10 @@ absl::flat_hash_map<std::string, absl::variant<int>> ConvertDictToMap( | |||
return map; | |||
} | |||
|
|||
// Override some upstream torch::lazy env vars for better performance. | |||
// Upstream lazy env vars defined in torch/csrc/lazy/core/config.h. | |||
void SetDefaultLazyEnvVars() { FLAGS_torch_lazy_handle_special_scalars = true; } |
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.
Thanks @ymwangg , do we know if it's always safe/better to use constants for special scalars?
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.
This has been what pytorch/xla has been doing, it is just when we migrate to LTC, we also need to enable a FLAG to make this behavior consistent.
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 guess the right thing to do here is to do FLAGS_torch_lazy_handle_special_scalars = ! xla::sys_util::GetEnvBool("XLA_NO_SPECIAL_SCALARS", false)
given in
https://github.com/pytorch/xla/blob/master/torch_xla/csrc/tensor.cpp#L257
we also have an env var to control this behavior
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 will merge this one to unblock aws folks, but submit a new one to fix it.
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.
Thanks for the catch! You are right I overlooked XLA_NO_SPECIAL_SCALARS. I can fix it here or in future PR, whichever works best for you.
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.
If you don't mind, can you update in this pr?
This PR fixed the issue described in #3942.
cc @JackCaoG