Added a '-seed' option to sample.lua to allow for an identical rerun.… #219
Conversation
… This option seeds the torch RNG with a selected integer value. Default behavior is unchanged - a random value is seeded if the option is omitted or set 0. The option honours the verbose flag and reports the seed, even the random seed, to permit an identical rerun if required. Please note - this does not affect the training at all - only the sample.lua
There was a problem hiding this comment.
Hi @turbotas, sorry for my slow response. This looks great, thanks :)
Unfortunately I don't have the ability to run torch-run at the moment, could someone please checkout this PR and verify it?
I've added a couple of comments for minor issues. Please also add an entry in docs/flags.md documenting this new feature.
| if opt.seed == 0 then | ||
| opt.seed = torch.random() | ||
| end | ||
| torch.manualSeed(opt.seed) |
There was a problem hiding this comment.
Might be simpler to force a manual seed only if -seed argument is set:
if <flag set>:
torch.manualSeed(...)
end
There was a problem hiding this comment.
I agree that this code looks odd. The reason I did this is to make sure that if you see some output that you would like to see again, and if you had the verbose flag set to report the seed, you can re-run by then including the seed - even if you didn't originally set the seed flag. So the code always sets a seed - using your provided value if you set one and a random number if you didn't (or specified zero!). I thought this was clever ;-)
There was a problem hiding this comment.
Ah okay that makes sense.
|
Perfect. I'll resolve your comments and resubmit.
…On 13 Mar 2018 5:26 pm, "Chris Cummins" ***@***.***> wrote:
***@***.**** requested changes on this pull request.
Hi @turbotas <https://github.com/turbotas>, sorry for my slow response.
This looks great, thanks :)
Unfortunately I don't have the ability to run torch-run at the moment,
could someone please checkout this PR and verify it?
I've added a couple of comments for minor issues. Please also add an entry
in docs/flags.md
<https://github.com/jcjohnson/torch-rnn/blob/master/doc/flags.md>
documenting this new feature.
------------------------------
In sample.lua
<#219 (comment)>:
> local opt = cmd:parse(arg)
local checkpoint = torch.load(opt.checkpoint)
local model = checkpoint.model
+if opt.seed == 0 then
What is someone wants to use a -seed 0? Is it possible to check if a flag
is explicitly set? If it is, then you should change the if conditional to
it; if not, then please document that the seed cannot be zero.
------------------------------
In sample.lua
<#219 (comment)>:
> local opt = cmd:parse(arg)
local checkpoint = torch.load(opt.checkpoint)
local model = checkpoint.model
+if opt.seed == 0 then
+ opt.seed = torch.random()
+end
+torch.manualSeed(opt.seed)
Might be simpler to force a manual seed only if -seed argument is set:
if <flag set>:
torch.manualSeed(...)
end
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#219 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ALi3noGdgmrGPbRau_Df9QsbnD7bv0gjks5teAEqgaJpZM4SaVLD>
.
|
|
I think I fixed comments and issues at commit 5d952d8 but not smart enough with git to know if I need to submit a new Pull request. |
|
Thanks @turbotas , yep the PR picked up your new commit. Can someone please run this branch and let me know it works as expected? It doesn't look like there's much room for anything expected, but would like a second opinion before merging. |
… This option seeds the torch RNG with a selected integer value.
Default sample.lua behavior is unchanged - a random value is seeded if the option is omitted or set 0.
The option honours the verbose flag and reports the seed, even the random seed, to permit an identical rerun if required.
Please note - this does not affect the training at all - only the sample.lua