A user can create a bundle that spans multiple files - #252
Conversation
| 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
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
| @@ -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)
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
| 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
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
| # 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?
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
importinside 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
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?
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
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