[Do not merge] Testing inference by seanshi-scale · Pull Request #107 · scaleapi/nucleus-python-client

seanshi-scale · 2021-08-24T23:23:15Z

Companion: https://github.com/scaleapi/models/pull/1847
Superseded by #203

yixu34

Think it's a bit too early to say more, but looks like the skeleton is coming together 💀

yixu34 · 2021-08-25T22:57:07Z

 dataclasses = { version = "^0.7", python = "^3.6.1, <3.7" }
 aiohttp = "^3.7.4"
 nest-asyncio = "^1.5.1"
+dill = "^0.3.0"


Btw my latest script used cloudpickle.

yixu34 · 2021-08-25T23:00:01Z

 mypy = "^0.812"
 coverage = "^5.5"
 pre-commit = "^2.12.1"
+torch = "1.7.1" # Test uploading model bundles only


Not sure if this too was temporary, but this brings up an interesting question around whether or not we want to package a certain version of pytorch in here. Seems like we should use whatever the user has installed locally.

Oh the reason this is in here is that I wanted to test uploading model bundles, so I used some stuff from pytorch. This is only a dependency of one test case, and is under [requirements-test]. I definitely think it would be weird/bad if nucleus-python-client (the package) had torch = 1.7.1 as a dependency.

yixu34

Will do another pass tomorrow.

yixu34 · 2022-01-14T01:20:21Z

@@ -0,0 +1,36 @@
+n.b. doesn't look like this readme gets copied into the sphinx docs


Don't we want it to?

yixu34 · 2022-01-14T01:20:38Z

+
+# Scale Deploy
+
+Shamelessly copied from PRFAQ :)


There is no shame in copy and paste 🌚

Well actually, I'd say we want a smaller blurb here for now. Here's what it looks like for Nucleus:

It's a bit odd to have content that sounds like a press release or marketing material in a README 🤔

yixu34 · 2022-01-14T01:23:31Z

+"""
+# Scale Deploy
+
+Shamelessly copied from PRFAQ :)


We can trim this down too.

yixu34 · 2022-01-14T01:23:59Z

        self, api_key: str, endpoint: str = HOSTED_INFERENCE_ENDPOINT
    ):
+        """
+        Initializes a HostedInference Client.


I think we actually want to rename all this public, user-facing stuff from hosted model inference -> deploy.

yixu34 · 2022-01-14T01:24:36Z

-        # Try to upload resulting predictions to nucleus
-
-    def _infer(
+    def infer(


I'd say predict is a more common name that appears in these kinds of frameworks.

yixu34 · 2022-01-14T05:17:53Z

+
+        Parameters:
+            endpoint_id: The id of the endpoint to make the request to
+            s3url: A url that points to a file containing model input.


For synchronous requests, I think we want to support inline payloads as well, i.e. you just send an image payload directly. This does get tricky with large payloads, though, where we'd probably want to stream the data in.

Anyway, I guess the point is that for synchronous endpoints:

We might not know the specifics yet for inline payloads, but:

We're pretty sure we will want them, which means that S3 URLs are not the only way, and should thus be Optional.

Another observation re: s3 urls is that we probably want to abstract this into a generic URI. This is because HMI can be self-hosted, in which case we'd want to support other schemes. Nucleus is able to assume S3 URLs because it's Scale-hosted.

yixu34 · 2022-01-14T05:21:42Z

+        resp = self.connection.get(route=f"task/result/{async_task_id}")
+        return resp["data"]
+
+    def batch_request(self, endpoint_id, s3urls: List[str]):


I made a comment on @Tianwei-She 's PR here: https://github.com/scaleapi/models/pull/2315#discussion_r784440984 - wondering if we can just get rid of this method.

yixu34 · 2022-01-14T05:22:24Z

+    A higher level abstraction for a Model Endpoint.
+    """
+
+    def __init__(self, endpoint_id, client):


These could use some type annotations and docstring.

yixu34 · 2022-01-14T05:25:30Z

@@ -0,0 +1,161 @@
+# This file contains all the Core Nucleus <-> HMI integrations


A couple of thoughts:

Maybe we should just rename this folder to deploy. This is just going to go into a separate release branch in the meantime anyway.

Regardless of what we name this folder (where the HMI/deploy stuff lives), I'd say that the integration code should go in an actual Nucleus folder. This way, it's clear that Nucleus depends on Deploy, without an inverse dependency too. Having this integration file here means that we can't just move this folder to a separate repo later, for instance.

yixu34 · 2022-01-14T05:27:32Z

+    hmi_client = HostedInference(
+        api_key="live_38aa469bc8ab436d89dce6e37e8d5258"
+    )
+    img_url = "s3://scale-ml-hosted-model-inference/tmp/hosted-model-inference-outputs/c3f3b5ed-f182-4fa1-bfa5-9b2e017feb74.pkl"


Probably best to remove any internal URLs in here. It's not the end of the world since's temporary and not a secret.

seanshi-scale · 2022-01-14T23:51:07Z

+        """
+        raise NotImplementedError
+
+    def edit_endpoint(self):


todo delete

seanshi-scale · 2022-01-19T00:33:50Z

See #203

seanshi-scale added 12 commits August 17, 2021 16:05

update gitignore

e36f6f2

function stubs for hosted model inference client

477dbd3

add dill to pyproject.toml

98f6df7

move to experimental folder

407d5ef

dump dill to s3 directly, add a test

c911bd7

update pyproject.toml to be less completely-non-portable

4d19371

add making requests to internal hosted inference

3c3c86d

.

dac750b

settle on request/response formats

799deab

return ModelBundle instead of string

22364a5

black

f7f0c37

black + pylint

3770b26

seanshi-scale requested a review from yixu34 August 25, 2021 22:08

seanshi-scale self-assigned this Aug 25, 2021

yixu34 reviewed Aug 25, 2021

View reviewed changes

seanshi-scale added 4 commits August 26, 2021 10:30

dill -> cloudpickle

938c880

remove /create from endpoint

2013041

start of create_model_endpoint

2dad6fe

remove currently unused model_name/reference id from request

43d68b4

seanshi-scale requested a review from syandroo August 27, 2021 00:04

seanshi-scale added 10 commits August 30, 2021 10:18

comment

e7cd983

some comments

29f77a4

start create_run_job

d2c6c37

.

ae760d5

.

4ffb8ba

live debugging changes :D, looks like create_model_endpoint works

cebd27e

test case for _nucleus_ds_to_s3url_list

fed81a3

model endpoint async job

e569f6f

temp test script + some code changes to accomodate rest api

00d3b53

can send tasks through and receive responses (links to pickles) back

156b1f6

seanshi-scale added 13 commits January 11, 2022 17:49

handle no gpu case on endpoint creation

4bb11c1

some docs

5ede121

more docs

098662c

move ModelBundle out into its own file

535bf71

readme

784e765

remove all nucleus-core references from ModelEndpoint

7a0409d

make ModelEndpoint use low level HMI endpoint

f58d5d6

add docs

bb5c312

stuff for docs

9995841

docs ++

e1baa79

docs +++

7c7ba0b

docs ++++

38f40a4

type annotations just cause

3542d60

yixu34 reviewed Jan 14, 2022

View reviewed changes

seanshi-scale added 7 commits January 14, 2022 10:05

shh

f78ac9b

trim intro blurb

f27c47f

rename HMI -> Deploy in python

1d98765

rename pt 2

706c9dc

rename some low level functions

840db97

respond to comments ++

f664a3a

add a find-packages file

687cf80

seanshi-scale commented Jan 14, 2022

View reviewed changes

seanshi-scale added 4 commits January 14, 2022 17:33

add finding model endpoint requirements

49be717

fix tests

fc4050d

see what the find_packages does

e305c5a

fix docs formatting

2168bc4

seanshi-scale force-pushed the seanshi/add-model-bundle-hosted-inference branch from 92aa45b to 2168bc4 Compare January 19, 2022 00:19

Merge branch 'master' into seanshi/add-model-bundle-hosted-inference

096a4cc

seanshi-scale closed this Jan 19, 2022

		@@ -0,0 +1,36 @@
		n.b. doesn't look like this readme gets copied into the sphinx docs

		@@ -0,0 +1,161 @@
		# This file contains all the Core Nucleus <-> HMI integrations

Uh oh!

Conversation

seanshi-scale commented Aug 24, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yixu34 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yixu34 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

seanshi-scale commented Jan 19, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

seanshi-scale commented Aug 24, 2021 •

edited

Loading