add cpu random Generator #26013
add cpu random Generator #26013
Conversation
|
Thanks for your contribution! |
There was a problem hiding this comment.
lgtm
this is a breaking change, the related OP should remove ctx.Attr<int>("seed") accordingly.
@yaoxuefeng6 will have instructions on how to fix the related OP.
@XiaoguangHu01 , @willthefrog , please have a look.
paddle/fluid/operators/randint_op.cc
Outdated
| } else { | ||
| unsigned int seed = static_cast<unsigned int>(ctx.Attr<int>("seed")); | ||
| std::minstd_rand engine; | ||
| if (seed == 0) { |
There was a problem hiding this comment.
what if use want to set seed to literal 0?
There was a problem hiding this comment.
Currently It is not allowed in paddle, and related doc note this to users. If it is necessary to allow this, we can fix it.
| namespace paddle { | ||
| namespace framework { | ||
|
|
||
| struct GeneratorState { |
There was a problem hiding this comment.
why note simply merge this into Generator as it is one-to-one "has a" relationship?
There was a problem hiding this comment.
To let State-related value be in this class. we can easily and clearly add things in this class later.
paddle/fluid/operators/randint_op.cc
Outdated
| } else { | ||
| unsigned int seed = static_cast<unsigned int>(ctx.Attr<int>("seed")); | ||
| std::minstd_rand engine; | ||
| if (seed == 0) { |
There was a problem hiding this comment.
what if use want to set seed to literal 0?
| Generator() { | ||
| GeneratorState default_gen_state_cpu; | ||
| default_gen_state_cpu.device = -1; | ||
| default_gen_state_cpu.current_seed = 34342423252; |
There was a problem hiding this comment.
default initialize with fixed seed?
There was a problem hiding this comment.
We should set a default one. maybe random?
There was a problem hiding this comment.
initialization with random seed seems more reasonable
PR types
Others
PR changes
Others
Describe
bak