Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: update generate_model_predict_response_for_dataset to use ray actors #24

Merged
merged 2 commits into from
Oct 11, 2023

Conversation

danielezhu
Copy link
Contributor

Issue #, if available:

Description of changes:
Move code that relates to Sagemaker sessions into a Ray Actor class inside generate_model_predict_response_for_dataset so that it doesn't run into issues serializing SSLContext objects (which happens if the session-creation logic is handled by the ModelRunner).

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@danielezhu danielezhu changed the title Update generate_model_predict_response_for_dataset to use Ray Actors fix: update generate_model_predict_response_for_dataset to use ray actors Oct 11, 2023
xiaoyi-cheng
xiaoyi-cheng previously approved these changes Oct 11, 2023
model.predictor = None
model.sagemaker_session = None

class ModelRunnerWrapper: # pragma: no cover
Copy link
Contributor

Choose a reason for hiding this comment

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

TODO: comments for context

keerthanvasist
keerthanvasist previously approved these changes Oct 11, 2023
@keerthanvasist keerthanvasist merged commit f867142 into main Oct 11, 2023
4 checks passed
@xiaoyi-cheng xiaoyi-cheng deleted the js_model_runner_update branch October 11, 2023 10:44
danielezhu added a commit that referenced this pull request Oct 11, 2023
…tors (#24)

* fix: update generate_model_predict_response_for_dataset to use ray actors

* fix: update actor pool strategy to use number of cpu cores instead of a static value

---------

Authored-by: Daniel Zhu <daniezh@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants