[Serve] Add deployment graph cookbook #24524

sihanwang41 · 2022-05-05T22:21:27Z

Why are these changes needed?

Provide more examples for user to use deployment graph api

Related issue number

Checks

I've run scripts/format.sh to lint the changes in this PR.
I've included any doc changes needed for https://docs.ray.io/en/master/.
I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
Testing Strategy
- Unit tests
- Release tests
- This PR is not tested :(

sihanwang41 · 2022-05-05T22:23:17Z

In the index, one more section for cookbook (for easy to get access)

in the deployment graph page, add a section to direct to the cookbook

cookbook page

doc/source/serve/deployment-graph-cookbook.md

jiaodong · 2022-05-05T23:44:59Z

doc/source/serve/deployment-graph-cookbook.md

+import ray
+from ray import serve
+from ray.experimental.dag.input_node import InputNode
+from ray.serve.drivers import DAGDriver


I think it's probably better to have a snippet at top of this doc as "imports" section so all the subsequent cells can be much shorter in length. We want to to the exact opposite of current e2e documentation

I will do this in further pr if we have lots of duplicates import (i think we will have :) ).

jiaodong · 2022-05-05T23:49:42Z

doc/source/serve/deployment-graph-cookbook.md

+      else:
+         prev_outputs[i] = nodes[i].forward.bind(prev_outputs[i - 1])
+
+   serve_dag = DAGDriver.options(route_prefix="/my-dag").bind(prev_outputs[-1])


i would suggest cutting this part completely and directly do dag.execute(0) and prints its output in the doc. The sequence of action we want user to take is Ray DAG -> Serve DAG where DAGDriver and serve.run() comes later when user needs http / serving / rolling updates.

Therefore i will be helpful to have sections like "How do I serve my DAG with HTTP" , "How do i add adapter / schema to my http input" and "How do I scale one node in my deployment graph"

prev_outputs[-1] is a bit confusing here as cookbook that aims for simplicity :)

Do you have suggestions what we should describe this scenario? I can add comments mention that "we need to use the last node to execute the dag" But let me know if you have good idea

doc/source/serve/deployment-graph-cookbook.md

jiaodong · 2022-05-05T23:52:46Z

doc/source/serve/deployment-graph-cookbook.md

+# 10 nodes chain
+num_nodes = 10
+nodes = [Model.bind(w) for w in range(num_nodes)]
+prev_outputs = [None] * num_nodes


i think it's worth having a "warning" as "anti-pattern" section for this part explaining why using a variable name and re-assign it in a loop is not the right way.

sihanwang41 · 2022-05-06T04:20:09Z

cookbook:

cookbook cont:

jiaodong · 2022-05-09T16:29:15Z

tip: After wheel is built, latest documentation on commit can be viewed on CI docs/readthedocs.org:ray , such as https://ray--24524.org.readthedocs.build/en/24524/serve/deployment-graph-cookbook.html#chain_nodes_same_class_different_args

doc/source/serve/deployment-graph-cookbook.md

jiaodong · 2022-05-09T16:40:14Z

doc/source/serve/deployment-graph-cookbook.md

+# 10 nodes chain in a line
+num_nodes = 10
+nodes = [Model.bind(w) for w in range(num_nodes)]
+outputs = [None] * num_nodes


we need very explicitly inline comment here (i would suggest even adding a WARNING section right underneath) about why this does not behave like a python variable that can be reused in a loop

Synced offline, leave it later.

doc/source/serve/deployment-graph-cookbook.md

jiaodong

This format looks much more self-contained and extendable :) really close now and just a few nits. I can take care of inline editing later on as we iterate on later parts.

doc/source/serve/deployment-graph/chain_nodes_same_class_different_args.md

maxpumperla · 2022-05-10T07:32:46Z

doc/source/_toc.yml

-          - file: serve/deployment-graph
+          - file: serve/deployment-graph/deployment-graph
+            sections:
+              - file: serve/deployment-graph/deployment-graph-e2e-tutorial


you know I'm against this "feature-first" approach to docs hierarchy, but I can't stop you guys from doing your thing.

We're referencing Ray dataset structure which i believe you contributed a lot to its format, open to changes :) iiuc you're suggesting we should bring that to first level user guide instead? I think that's better format too to avoid nesting docs, but meanwhile Deployment graph was just in alpha stage but i haven't found a structure that satisfies both yet. What do you think is more reasonable?

I'm working on Serve doc restructuring. This structure will be transient for now.

Thanks for clarifying, @simon-mo . We don't have to change anything for this PR, just wanted to chime in.

@jiaodong Generally, users are interested in their workloads and workflows, rather than specific features of a library (that they often don't know yet and won't click on). A user guide / cookbook for this is great, but I wouldn't put it top level. To me there shouldn't be a "Deployment Graph" doc there at all, as I find the structure a little confusing.

Where should I go to learn about deploying my models first? Core API: Deployments, Calling Deployments, Serving ML Models, Deployment Graph, or Deploying Ray Serve? They all sound very similar and I don't want to read through 5 docs first to understand what I need, because I might only have 10 minutes. All relatively easy to streamline, I think. No worries there.

Thanks for the inputs Max, it makes a lot of sense. As we chatted earlier in March, Serve team should re-structure our docs to better organize our content on this and it's happening now, please don't hesitate to step up and give us suggestions like this :)

For this PR in particular, Sihan is ramping on the context to make progress, but I will self-assign and ensure syncing with Simon to properly organize them as part of the docs restructure effort.

jiaodong

Sorry i should have mentioned this earlier for the image file please commit it to separate repo to make the size of ray repo smaller: https://github.com/ray-project/images/tree/master/docs, you should be able to push it directly and link it via the url of its raw content.

The content and structure looks good to me for the first page. Please rebase and resolve a small merge conflict and let's aim to merge it tomorrow morning

jiaodong

Letting @simon-mo to take another look

simon-mo · 2022-05-11T17:31:10Z

Hmm looks like this snippet is still not tested. I think we need to add a py_test target at https://github.com/ray-project/ray/blob/master/python/ray/serve/BUILD#L458-L464

simon-mo

LGTM as long as test passes

simon-mo · 2022-05-11T23:24:51Z

//python/ray/serve:deployment_graph_same_class_different_args PASSED in 14.7s

merging

sihanwang41 assigned jiaodong May 5, 2022

jiaodong reviewed May 5, 2022

View reviewed changes

jiaodong reviewed May 9, 2022

View reviewed changes

jiaodong added the @author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer. label May 9, 2022

jiaodong reviewed May 10, 2022

View reviewed changes

jiaodong assigned edoakes and simon-mo May 10, 2022

maxpumperla reviewed May 10, 2022

View reviewed changes

doc/source/serve/deployment-graph/chain_nodes_same_class_different_args.md Show resolved Hide resolved

maxpumperla reviewed May 10, 2022

View reviewed changes

jiaodong reviewed May 11, 2022

View reviewed changes

sihanwang41 added 5 commits May 10, 2022 18:29

[Serve] Add deployment graph cookbook

b149556

update & address comments

a4a3b93

restructure the doc

e35fd88

Put the code into doc_code & address comments

a26f03a

move image to different repo

86c8cba

sihanwang41 force-pushed the graph_cookbook branch from 431eec0 to 86c8cba Compare May 11, 2022 01:34

jiaodong approved these changes May 11, 2022

View reviewed changes

Add test into bazel

1a3a05b

simon-mo approved these changes May 11, 2022

View reviewed changes

simon-mo merged commit c5bfe1d into ray-project:master May 11, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Serve] Add deployment graph cookbook #24524

[Serve] Add deployment graph cookbook #24524

sihanwang41 commented May 5, 2022

sihanwang41 commented May 5, 2022 •

edited

jiaodong May 5, 2022

sihanwang41 May 6, 2022

jiaodong May 5, 2022

jiaodong May 5, 2022

sihanwang41 May 6, 2022

jiaodong May 5, 2022

sihanwang41 commented May 6, 2022 •

edited

jiaodong commented May 9, 2022

jiaodong May 9, 2022

sihanwang41 May 10, 2022

jiaodong left a comment

maxpumperla May 10, 2022

jiaodong May 10, 2022

simon-mo May 10, 2022

maxpumperla May 10, 2022

jiaodong May 11, 2022

jiaodong left a comment

jiaodong left a comment

simon-mo commented May 11, 2022

simon-mo left a comment

simon-mo commented May 11, 2022

[Serve] Add deployment graph cookbook #24524

[Serve] Add deployment graph cookbook #24524

Conversation

sihanwang41 commented May 5, 2022

Why are these changes needed?

Related issue number

Checks

sihanwang41 commented May 5, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sihanwang41 commented May 6, 2022 • edited

jiaodong commented May 9, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jiaodong left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jiaodong left a comment

Choose a reason for hiding this comment

jiaodong left a comment

Choose a reason for hiding this comment

simon-mo commented May 11, 2022

simon-mo left a comment

Choose a reason for hiding this comment

simon-mo commented May 11, 2022

sihanwang41 commented May 5, 2022 •

edited

sihanwang41 commented May 6, 2022 •

edited