Repository navigation
Conversation
6960e40 to
84bb979
Compare
ea1f87a to
b6a62eb
Compare
This comment has been minimized.
This comment has been minimized.
b6a62eb to
6034f5f
Compare
6ee0251 to
809b5c2
Compare
6034f5f to
1f9a721
Compare
809b5c2 to
2dca807
Compare
1f9a721 to
6df6a5f
Compare
2dca807 to
fc5d9f9
Compare
6df6a5f to
6f734af
Compare
6f734af to
de41052
Compare
fc5d9f9 to
2302338
Compare
171ff1d to
488d029
Compare
2302338 to
c388455
Compare
488d029 to
f52ef49
Compare
c388455 to
83faf53
Compare
f52ef49 to
1c7603b
Compare
83faf53 to
ef94bc2
Compare
1c7603b to
a4812f1
Compare
38615b3 to
f6058a8
Compare
c06bba2 to
76e03a8
Compare
f6058a8 to
688dfc9
Compare
76e03a8 to
d6d452e
Compare
688dfc9 to
d30320b
Compare
d6d452e to
11e7454
Compare
| "classifier": self._classifier, | ||
| "generator": decoder_stage, | ||
| } | ||
| plan = compile_workflow(define_workflow()) |
There was a problem hiding this comment.
Does user have to author this worker.py for every single ensemble pipeline?
If so, this file seems to contain a lot of boilerplate code.
It might be a cleaner interface if a user only needs to author L87-L105 (the top portion of this start() function) to register the custom stages and workflow.
There was a problem hiding this comment.
makes sense. will take this into consideration
| backend, | ||
| resolved_model_config, | ||
| config.engine_args, | ||
| ) |
There was a problem hiding this comment.
given an output of encoder, users may need to pass different kinds of Prompt into engine (vllm in this case).
e.g.
- it may be a
EmbedPromptfor LLM model - it may be a
TokenPromptfor VLM model (because Qwen VL requiresgrid_thwalongside embedding)
I am seeing different cases where one of these prompts is needed.
| merged_ensemble["classifier_scores"] = dict(scores) | ||
| merged_engine_data["ensemble"] = merged_ensemble | ||
| adapted["engine_data"] = merged_engine_data | ||
| return adapted |
There was a problem hiding this comment.
If I'm not mistaken, this function is the post-processing step that stitches multiple outputs from different stages together, correct?
Even though this is pure Python logic without ML, would it be OK to model this step as a stage as well? That way, nothing lives outside of the workflow.
There was a problem hiding this comment.
correct. it's totally doable to model it inside stage.
d30320b to
d08f9fc
Compare
586253d to
4590bbf
Compare
fe3c7f8 to
59f743f
Compare
4590bbf to
092393e
Compare
092393e to
f187ca9
Compare
f187ca9 to
940aa08
Compare
c01209e to
fba8b57
Compare
There was a problem hiding this comment.
From user-interface perspective, the workflow approach seems cleaner, assuming this doesn't require a custom WorkflowEndpointHandler implementation like BespokeEnsembleHandler.
From functionality perspective, I cannot tell whether either approach can support remote stage calling in the future. For example, in bespoke approach, does the classifier_task have to be a local, in-process call? In workflow approach, does classifier stage have to be a local, in-process call?
I'm wondering if we can fuse the two approaches into one:
- Let user-interface use the workflow approach - define graph topology without
asyncio.create_task()boilerplate. - Let the Dynamo-provided
WorkflowEndpointHandlerbe the default handler for workflow execution, and if users want to customize the execution logic, like adding prioritization or throttling, they can overrideWorkflowEndpointHandler, and at that point, the custom handler will look something likeBespokeEnsembleHandler.
| custom_template_path=str(CHAT_TEMPLATE), | ||
| ignore_weights=True, | ||
| ) | ||
| await endpoint.serve_endpoint(WorkflowEndpointHandler(orchestrator).generate) |
There was a problem hiding this comment.
Where is WorkflowEndpointHandler defined?
Does user have to implement custom WorkflowEndpointHandler for each workflow?
There was a problem hiding this comment.
Where is WorkflowEndpointHandler defined?
Does user have to implement custom WorkflowEndpointHandler for each workflow?
No, WorkflowEndpointHandler kicks off orchestrator which executes users' workflow.
the assumption is correct Inline- and Remote-
Existing Dynamo primitives already support it remote stage calling. both Dynamo Primitivetl;dr users' Python can
Workflow (inline- and remote- switch)the workflow abstract the above logic so that users only need to
This would allow users to switch between inline- and remote- easily. Does this abstraction make things more complicated than helpful? |
agreed.
|
The Workflow abstraction hides the execution details/complexity, giving users a clean way to define graph topology; it also decouples graph definition and instance binding nicely - reminding me of "dependency injection" design pattern in Java world, so I would definitely prefer using the WF approach. |
Based on #13293.
Why
Dynamo endpoints already let applications build this ensemble manually, so the workflow API should demonstrate its value against an equivalent baseline. Both variants share the inline components, stock vLLM worker, and the same
GenerateEndpointInvokerfor request validation, stream folding, and remote-call cancellation. This isolates the comparison to orchestration ownership: the workflow runtime owns graph scheduling and binding, while the bespoke handler owns coroutine and sibling-task lifecycle.Effect
What Change
classDiagram WorkflowOrchestrator *-- StageDispatcher : schedules graph StageDispatcher ..> GenerateEndpointInvoker : invokes remote Generate BespokeEnsembleHandler ..> GenerateEndpointInvoker : invokes same adapter GenerateEndpointInvoker ..> DynamoClient : calls discovered endpoint BespokeEnsembleHandler --> EncoderStage : schedules inline note for WorkflowOrchestrator "Owns authored graph execution" note for BespokeEnsembleHandler "Owns equivalent task orchestration" note for GenerateEndpointInvoker "Shares validation, folding, and call cancellation"Test Plan