Skip to content
This repository has been archived by the owner on Nov 3, 2023. It is now read-only.

Fix seed from opt in rng sampler of multiwoz_v22 #4736

Merged
merged 23 commits into from
Aug 22, 2022

Conversation

prajjwal1
Copy link
Contributor

Patch description
Since the support for setting seed in train_model.py has been added, we can let user fix the seed in dataset sampler also.

Testing steps

Other information

Copy link
Contributor

@moyapchen moyapchen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See inline comment; Accepting under assumption recommendation there taken

def __init__(self, opt: Opt, shared=None, *args, **kwargs):
self.opt = opt
self.fold = opt["datatype"].split(":")[0]
opt["datafile"] = self.fold
self.dpath = os.path.join(opt["datapath"], "multiwoz_v22")
self.id = "multiwoz_v22"
if self.opt["seed"]:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would recommend using self.opt.get("seed") to be safe, but otherwise looks fine.

@moyapchen
Copy link
Contributor

Actually thinking about this slightly more - nothing actually adds a flag for "seed" to the argparser. How are you passing the "seed" argument down?

@prajjwal1
Copy link
Contributor Author

prajjwal1 commented Aug 11, 2022

Thanks for replying @moyapchen. I now perform indexing with .get(). Even I realized the issue you raised, I thought it would come from train_model.py, but seems like it has to come from Teacher, so the argument for seed is propagated from Teacher. Is this fine now ?

parlai/core/teachers.py Outdated Show resolved Hide resolved
parlai/core/teachers.py Outdated Show resolved Hide resolved
@klshuster klshuster merged commit a695ba8 into facebookresearch:main Aug 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants