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

[BB2] Handle Distributed #4023

Merged
merged 1 commit into from Sep 16, 2021
Merged

[BB2] Handle Distributed #4023

merged 1 commit into from Sep 16, 2021

Conversation

klshuster
Copy link
Contributor

Patch description
Make sure we don't directly access functions from self.model if we're in distributed mode. Fixes issue pointed out in #3872

Follows very closely the solution in #3787

Testing steps
Verified that bb2 tests still pass.

@@ -507,7 +515,7 @@ def _set_query_vec(self, observation: Message) -> Message:
)
if self.add_person_tokens:
query_str = self._remove_person_tokens(query_str)
observation['query_vec'] = self.model.tokenize_query(query_str)
observation['query_vec'] = self.model_api.tokenize_query(query_str)
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be self.model_api().tokenize_query(query_str)?

Copy link
Contributor

Choose a reason for hiding this comment

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

idts, model_api is a @property

Kurt, maybe we should bump this all the way up to TA. We have this module stuff everywhere

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, yeah right. I missed that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

bump up to TA

We would need to think of an elegant solution so that we can still call self.model() directly (which does work in distributed). We could solve it with model_api if you think that's appropriate

Copy link
Contributor

Choose a reason for hiding this comment

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

you're right, nvm

@klshuster klshuster merged commit 6f16710 into main Sep 16, 2021
@klshuster klshuster deleted the bb2_multiprocessing branch September 16, 2021 22:15
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.

None yet

4 participants