Update torch.range warning message regarding the removal version number #43569
Conversation
|
I do have a PR removing |
|
@zasdfgbnm Thanks for the pointer. Perhaps we should just modify the warning message in this PR? |
|
@xuhdev Yes, agree. This PR LGTM |
💊 CI failures summary and remediationsAs of commit b413a7f (more details on the Dr. CI page): ✅ None of the CI failures appear to be your fault 💚
🚧 1 fixed upstream failure:These were probably caused by upstream breakages that were already fixed.
Please rebase on the
|
|
Let me follow-up and see if this is removable now. |
mruberry
added
triaged
There was a problem hiding this comment.
Updating this warning message is a good idea. I had the opportunity to review the history of this deprecation with @gchanan and we'd like to clarify why the function is being deprecated too, and that's because it's inconsistent with Python's range function (see the ancient #733).
What about a sentence like, "torch.range is deprecated and will be removed in a future release because its behavior is inconsistent with Python's range builtin. Use torch.arange, which produces values in [start, end), instead."
We could also update the warning in the documentation for torch.range to something like:
"This function is deprecated and will be removed in a future release because its behavior is inconsistent with Python's range builtin. Use torch.arange, which produces values in [start, end), instead."
Do you think that would improve these warnings, @xuhdev?
There was a problem hiding this comment.
Definitely, I agree the new message is more informative. In addition, I also added that torch.arange runs faster as an additional incentive. (arange is vectorized but range is not)
mruberry
left a comment
mruberry
left a comment
There was a problem hiding this comment.
Thanks for taking care of this, @xuhdev! I slightly tweaked your wording to remove the claim that arange is faster, just because performance is so highly variable and system dependent. I actually checked and from code review and empiric timings arange IS faster, though.
facebook-github-bot
left a comment
facebook-github-bot
left a comment
There was a problem hiding this comment.
@mruberry has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
5 participants
torch.rangestill hasn't been removed way after version 0.5. This PR fixes the warning message. Alternatively, we can removetorch.range.