A user can create a bundle that spans multiple files - #252
Conversation
Sorry, something went wrong.
| rich = "^10.15.2" | ||
| shellingham = "^1.4.0" | ||
| scikit-learn = ">=0.24.0" | ||
| cloudpickle = "^1.6.0" |
There was a problem hiding this comment.
Does this mean we'll have to pin the version of cloudpickle inside of the async/sync inference endpoints to 2.0.0? Not entirely sure actually
Sorry, something went wrong.
There was a problem hiding this comment.
Nope!
Technically to my understanding and exploring the cloudpickle source code, you can use even simple pickle to deserialize
Sorry, something went wrong.
| @@ -253,6 +253,46 @@ packaging = "*" | |||
| six = ">=1.9.0" | |||
| webencodings = "*" | |||
|
|
|||
There was a problem hiding this comment.
ty for checking in poetry.lock lol (diego told me about this, I'm guessing he asked you as well)
Sorry, something went wrong.
There was a problem hiding this comment.
yeah, there was a weird bug with other packages. I guess we just need to call poetry add <package> instead of doing it manually in pyproject.toml
Sorry, something went wrong.
| model: Optional[DeployModel_T] = None, | ||
| load_model_fn: Optional[Callable[[], DeployModel_T]] = None, | ||
| bundle_url: Optional[str] = None, | ||
| globals_copy: Optional[Dict[str, Any]] = None, |
There was a problem hiding this comment.
Is there a reason that the client has to pass in globals().copy(), or can we get the value of that from inside this function? I think if there isn't a reason for the client to pass this in then we shouldn't make them do that
Sorry, something went wrong.
There was a problem hiding this comment.
to my understanding no... There are some problems with using iterating globals() i.e. for k, v in globals.items() because size of globals changes while iterating. If you called globals from outside of the function and pass it as globals_copy then it's fine.
To answer you question: no it is not needed to .copy() it (I think). But I used this name to keep the same naming convention as in https://github.com/scaleapi/nucleus-python-client/blob/hmi-release/nucleus/deploy/find_packages.py#L323
Sorry, something went wrong.
| # Avoid recursion | ||
| # register_pickle_by_value does not work properly with itself | ||
| continue | ||
| cloudpickle.register_pickle_by_value(module) |
There was a problem hiding this comment.
@yixu34 did this cloudpickle.register_pickle_by_value thing end up working? Thought it didnt?
Sorry, something went wrong.
There was a problem hiding this comment.
I checked it with an experiment: having some code in another file different from where I defined the bundle.
There are some limits to this approach:
- if you do import inside the bundle function, it won't work
- you cannot use non-module imports aka from foo import woo. This is a known issue, but I thought solving it is a little overkill at this point. We'd need to go to low level
Sorry, something went wrong.
There was a problem hiding this comment.
hmm so it works just that the only limitation is you have to import the full module and outside the function?
Sorry, something went wrong.
There was a problem hiding this comment.
@syandroo given that they say on cloudpickle page: "Note that this feature is still experimental" I wouldn't be very hopeful :)
At least a simple use case of multiple .py files works fine.
I guess the next step is to try on one of the scale ml projects
Sorry, something went wrong.
https://app.shortcut.com/scaleai/story/396138/a-user-can-create-a-bundle-that-spans-multiple-files
Users can pickle bundles that rely on external modules. It's done by passing globals() value