Skip to content

add predict_fn_or_cls as a bundle #246

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.

Sign up for GitHub

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

Merged
ivan-khomyakov-sc merged 13 commits into from
Mar 12, 2022
Merged

add predict_fn_or_cls as a bundle #246

Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
926e523
add predict_fn_or_cls argument; add cluster parameters
ivan-khomyakov-sc Mar 4, 2022
69595ea
black
ivan-khomyakov-sc Mar 4, 2022
54b4443
add code bundle
ivan-khomyakov-sc Mar 4, 2022
caaa2ac
Merge branch 'hmi-release' into ivan_k/upload_class_bundle
ivan-khomyakov-sc Mar 5, 2022
aae49d3
rm aws_role and results_s3_bucket as params
ivan-khomyakov-sc Mar 7, 2022
d79f133
fix tests
ivan-khomyakov-sc Mar 7, 2022
1a9176f
black
ivan-khomyakov-sc Mar 8, 2022
28ac5fa
clarify docstring; check if model bundle is correct
ivan-khomyakov-sc Mar 11, 2022
6641c17
use a single bundle var
ivan-khomyakov-sc Mar 12, 2022
5538b85
black
ivan-khomyakov-sc Mar 12, 2022
1b2a6a2
Merge branch 'hmi-release' into ivan_k/upload_class_bundle
ivan-khomyakov-sc Mar 12, 2022
51b6ff5
disable too-many-branches linter
ivan-khomyakov-sc Mar 12, 2022
dc21407
black
ivan-khomyakov-sc Mar 12, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 37 additions & 15 deletions nucleus/deploy/client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,8 +198,12 @@ def create_model_bundle_from_dir(
def create_model_bundle(
self,
model_bundle_name: str,
load_predict_fn: Callable[[DeployModel_T], Callable[[Any], Any]],
env_params: Dict[str, str],
*,
load_predict_fn: Optional[
Callable[[DeployModel_T], Callable[[Any], Any]]
] = None,
predict_fn_or_cls: Optional[Callable[[Any], Any]] = None,

@yixu34 yixu34 Mar 11, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Overall I think this is fine for now, but we might as well start deprecating load_predict_fn later.

@yixu34 yixu34 Mar 11, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Oh I guess same comment as https://github.com/scaleapi/models/pull/2684/files#r820019055, basically.

requirements: Optional[List[str]] = None,
model: Optional[DeployModel_T] = None,
load_model_fn: Optional[Callable[[], DeployModel_T]] = None,
Expand All @@ -208,18 +212,19 @@ def create_model_bundle(
) -> ModelBundle:
"""
Grabs a s3 signed url and uploads a model bundle to Scale Deploy.
A model bundle consists of a "load_predict_fn" and exactly one of "model" or "load_model_fn", such that
load_predict_fn(model)
or
load_predict_fn(load_model_fn())
returns a function predict_fn that takes in model input and returns model output.
Pre/post-processing code can be included inside load_predict_fn/model.

A model bundle consists of exactly {predict_fn_or_cls}, {load_predict_fn + model}, or {load_predict_fn + load_model_fn}.
Pre/post-processing code can be included inside load_predict_fn/model or in predict_fn_or_cls call.

Parameters:
model_bundle_name: Name of model bundle you want to create. This acts as a unique identifier.
predict_fn_or_cls: Function or a Callable class that runs end-to-end (pre/post processing and model inference) on the call.
I.e. `predict_fn_or_cls(REQUEST) -> RESPONSE`.
model: Typically a trained Neural Network, e.g. a Pytorch module
load_model_fn: Function that when run, loads a model, e.g. a Pytorch module
load_predict_fn: Function that when called with model, returns a function that carries out inference

@yixu34 yixu34 Mar 4, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It looks like the original type annotation of load_predict_fn was a Callable: Callable[[DeployModel_T], Callable[[Any], Any]]. In that case, maybe we don't need to create a new predict_fn_or_cls field, since load_predict_fn was already forwards compatible with the way you've specified the Bundle class with a __call__ method?

@ivan-khomyakov-sc ivan-khomyakov-sc Mar 4, 2022

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

They have different usage:
a. predict_fn_or_cls

foo("woo") # returns inference result;
client.upload_bundle(predict_fn_or_cls=foo)

b. load_predict_fn

foo("woo") # returns inference result;

# incorrect:
client.upload_bundle(load_predict_fn=foo)

# correct:
def load_predict_fn(model):
    def predict(args):
          return foo(args)
    return predict

@ivan-khomyakov-sc ivan-khomyakov-sc Mar 4, 2022

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I mean predict_fn_or_cls is a function/class that you can use directly in the service. load_predict_fn on the other hand is a factory that produces a callable object.

@yixu34 yixu34 Mar 11, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Oh I see, the absence or presence of the load prefix can serve as a distinction as to whether or not it's directly used or not 👌🏻

I.e. `load_predict_fn(model) -> func; func(REQUEST) -> RESPONSE`
load_model_fn: Function that when run, loads a model, e.g. a Pytorch module
I.e. `load_predict_fn(load_model_fn()) -> func; func(REQUEST) -> RESPONSE`
bundle_url: Only for self-hosted mode. Desired location of bundle.
Overrides any value given by self.bundle_location_fn
requirements: A list of python package requirements, e.g.
Expand All @@ -235,12 +240,18 @@ def create_model_bundle(
"tensorflow_version": Version of tensorflow, e.g. "2.3.0". Only applicable if framework_type is tensorflow
globals_copy: Dictionary of the global symbol table. Normally provided by `globals()` built-in function.
"""
# TODO(ivan): remove `disable=too-many-branches` when get rid of `load_*` functions
# pylint: disable=too-many-branches

if (model is not None and load_model_fn is not None) or (
model is None and load_model_fn is None
):
check_args = [
predict_fn_or_cls is not None,
load_predict_fn is not None and model is not None,
load_predict_fn is not None and load_model_fn is not None,
]

if sum(check_args) != 1:
raise ValueError(
"Exactly one of model and load_model_fn should be non-None"
"A model bundle consists of exactly {predict_fn_or_cls}, {load_predict_fn + model}, or {load_predict_fn + load_model_fn}."
)
# TODO should we try to catch when people intentionally pass both model and load_model_fn as None?

Expand All @@ -266,23 +277,34 @@ def create_model_bundle(
continue
cloudpickle.register_pickle_by_value(module)

bundle: Union[
Callable[[Any], Any], Dict[str, Any], None
] # validate bundle
bundle_metadata = {}
# Create bundle
if model is not None:
if predict_fn_or_cls:
bundle = predict_fn_or_cls
if inspect.isfunction(predict_fn_or_cls):
source_code = inspect.getsource(predict_fn_or_cls)
else:
source_code = inspect.getsource(predict_fn_or_cls.__class__)
bundle_metadata["predict_fn_or_cls"] = source_code
elif model is not None:
bundle = dict(model=model, load_predict_fn=load_predict_fn)
bundle_metadata["load_predict_fn"] = inspect.getsource(
load_predict_fn
load_predict_fn # type: ignore
)
else:
bundle = dict(
load_model_fn=load_model_fn, load_predict_fn=load_predict_fn
)
bundle_metadata["load_predict_fn"] = inspect.getsource(
load_predict_fn
load_predict_fn # type: ignore
)
bundle_metadata["load_model_fn"] = inspect.getsource(
load_model_fn # type: ignore
)

serialized_bundle = cloudpickle.dumps(bundle)

if self.is_self_hosted:
Expand Down