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

[BlenderBot2] A hybrid mode (skip search when needed) #4221

Merged
merged 9 commits into from
Dec 9, 2021
Merged

Conversation

jxmsML
Copy link
Contributor

@jxmsML jxmsML commented Dec 1, 2021

Patch description
Pass a new batch field skip_search to classify_retrieval: to skip search if that field is true.

This can be useful for example, when gold knowledge is prepended to the text.

Testing steps

Other information

@jxmsML jxmsML changed the title [BlenderBot2] A hybrid mode (skip search when gold knowledge prepended to the text) [BlenderBot2] A hybrid mode (skip search when needed) Dec 1, 2021
@@ -181,7 +181,7 @@ def __init__(self, opt: Opt):
)
assert isinstance(base_agent, TorchAgent)
self.agents = [base_agent]
bsz = max(opt.get('batchsize', 1), opt.get('eval_batchsize', 1))
bsz = max(opt.get('batchsize') or 1, opt.get('eval_batchsize') or 1)
Copy link
Contributor

@mojtaba-komeili mojtaba-komeili Dec 1, 2021

Choose a reason for hiding this comment

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

Oddly, I was getting this error when trying this agent. This change was a solution to avoid it.

Screen Shot 2021-12-01 at 11 24 42 AM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmm interesting, I thought anything return from opt.get(XXX, 1) would never be None

Copy link
Contributor

Choose a reason for hiding this comment

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

it can be None if the arg is in opt but no value is given

@@ -894,6 +914,13 @@ def build_model(self) -> Union[BlenderBot2FidModel, T5BlenderBot2FidModel]:
return model


class BlenderBot2SearchQueryFiDAgent(BlenderBot2FidAgent):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

@klshuster klshuster left a comment

Choose a reason for hiding this comment

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

cool, looks good to me!

yield message, episode_started


class WizardDialogGoldKnowledgeTeacher(WizardDialogTeacher):
def __init__(self, opt, shared=None):
super().__init__(opt, shared=shared)
self.id = 'WizardDialogGoldKnowledgeTeacher'
Copy link
Contributor Author

@jxmsML jxmsML Dec 2, 2021

Choose a reason for hiding this comment

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

ah thanks for adding this, otherwise the train_model always flags this red warning ;)

@jxmsML
Copy link
Contributor Author

jxmsML commented Dec 9, 2021

teacher tests seem to pass now! thanks god will merge it.

@jxmsML jxmsML merged commit 8f9df51 into main Dec 9, 2021
@jxmsML jxmsML deleted the bbgold_hybrid branch December 9, 2021 16:39
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