Add random state #204
Add random state #204
Conversation
katxiao
requested review from
pvk-developer and
csala
and removed request for
a team
katxiao
force-pushed
the
add-random-state
branch
2 times, most recently
from
2235d35
to
6a855bb
Compare
There was a problem hiding this comment.
Added a few comments. The ones added to sdv-dev/Copulas#313 should be considered as well.
Also, I think that integration tests should be added to ensure that the effect of setting the seed is the expected one.
ctgan/synthesizers/base.py
Outdated
| import torch | ||
|
|
||
|
|
||
| @contextlib.contextmanager | ||
| def set_random_state(random_state, set_model_random_state): |
There was a problem hiding this comment.
Since this uses torch, should we be setting the torch seed too?
There was a problem hiding this comment.
Other than the torch seed, which concerns me as well, I think this looks good. Here is a resource regarding randomness on pytorch : https://pytorch.org/docs/stable/notes/randomness.html
katxiao
force-pushed
the
add-random-state
branch
from
110f3f6
to
10bbcd5
Compare
katxiao
force-pushed
the
add-random-state
branch
from
10bbcd5
to
8243fbf
Compare
Codecov Report
@@ Coverage Diff @@
## master #204 +/- ##
==========================================
+ Coverage 36.09% 38.73% +2.63%
==========================================
Files 10 10
Lines 676 710 +34
==========================================
+ Hits 244 275 +31
- Misses 432 435 +3
Continue to review full report at Codecov.
|
katxiao
force-pushed
the
add-random-state
branch
from
3f2ded9
to
807593a
Compare
Add random state field to CTGAN and TVAE models.
random_stateon base classrandom_statedecorator to use around sampling methods