[Chunk Teacher] Allow multi-turn #2805
Conversation
|
Your PR contains a change to a task. Please paste the results of the following command into a comment: python tests/datatests/test_new_tasks.py |
| @@ -2021,6 +2024,7 @@ | |||
| self._enqueue_request() | |||
|
|
|||
| self.episode_done = True | |||
| self.last_queue_output = None | |||
|
|
|||
There was a problem hiding this comment.
Couldn’t you just use self.opt?
parlai/core/teachers.py
Outdated
| queue_output = self.samples.get() | ||
| if queue_output is None: | ||
| return None | ||
| if self.episode_done: |
There was a problem hiding this comment.
Would prefer this private I think
There was a problem hiding this comment.
What do you mean? I actually didn't add this varialbe, I believe it's something used by FixedDialogTeacher
There was a problem hiding this comment.
we could just make this self._episode_done since this teacher doesn't actually use the self.episode_done from FixedDialogTeacher right? or does it?
| @@ -151,7 +153,7 @@ def load_from_chunk(self, chunk_idx: int) -> List[Tuple[str, str]]: | |||
|
|
|||
| return output | |||
|
|
|||
| def create_message(self, queue_output: Tuple[str, ...]) -> 'Message': | |||
parlai/core/teachers.py
Outdated
| queue_output = self.samples.get() | ||
| if queue_output is None: | ||
| return None | ||
| if self.episode_done: |
There was a problem hiding this comment.
we could just make this self._episode_done since this teacher doesn't actually use the self.episode_done from FixedDialogTeacher right? or does it?
| @@ -575,7 +577,7 @@ def load_from_chunk(self, chunk_idx: int): | |||
| output.append((text, resp)) | |||
| return output | |||
|
|
|||
| def create_message(self, sample_item): | |||
| def create_message(self, sample_item, entry_idx=0): | |||
There was a problem hiding this comment.
it'd probably be nice to have a MultiturnChunkyTeacher as well in the integration tests 😄
|
Or, perhaps @stephenroller knows better (I have personally never used |
|
I've tested all of the tasks publicly and internally with these changes... I have a PR in internal that's ready to land once this lands. I'll make episode_done private and land |
Patch description
Some changes to Chunk Teacher: