Let set_rng_state and get_rng_state accept string parameter #23448
Conversation
pytorchbot
added
the
module: cuda
xuhdev
changed the title
xuhdev
added
the
module: random
|
Waiting on CI |
There was a problem hiding this comment.
@soumith is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
Nice. Another (possibly better) way is to make the default argument the “cuda” string. :) |
Good point! I've updated the PR so string is now also accommodated. |
Currently the default device of set_rng_state and get_rng_state is
torch.device('cuda'). But this is only evaluated once when the function
definition is executed. In other words, the default value would not be
correct if the user calls torch.set_device() to a different device than
the one when the function definitions of set_rng_state and get_rng_state
are executed. See
https://docs.python.org/3.7/reference/compound_stmts.html#function-definitions
> Default parameter values are evaluated from left to right when the function definition is executed. This means that the expression is evaluated once, when the function is defined, and that the same **pre-computed** value is used for each cal
|
Wait I’m actually a little bit confused. torch.device(“cuda”) returns a device object with device index None, I.e., it always refers to the current device. Are you sure that the old default values have problems? |
I'm not sure whether it always refers to the current device, which is not clearly written in torch.device doc. If it is so, I don't think there will be any issue with the default param and I can edit the message of this PR to "set_rng_state and get_rng_state now accept string param". In either case, I'll also send a PR to clarify this in the doc. |
|
Okay, yeah that sounds good to me. :) You can use this function to get the device index: Line 5 in f0ebf76 |
xuhdev
changed the title
|
@ssnl Updated! |
There was a problem hiding this comment.
@soumith has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
Can we switch to |
|
@ssnl Are you talking about this line |
Currently set_rng_state and get_rng_state do not accept string as their parameters. This commit let them accept strings.