[CV2-5117] spec doc describing batch format only #116
Conversation
skyemeedan
requested review from
DGaffney and
computermacgyver
as code owners
|
This is the same document reviewed in previous PR, but without the refactor |
| * Models that don't handle requests in parallel need to implement batch format (but can give errors if they recieve more than 1 item) | ||
| * "top level" elements in the payload (outermost dictionary keys) are processed and controlled by the Presto system | ||
| * Elements inside parameters, and inside individual input items are passed through to the model | ||
| * From the perspective of the calling service, all the of the items in a single batch call must be dispatched to the same Presto model service. (don't mix items for paraphrase and classycat it the same call) |
There was a problem hiding this comment.
as I had mentioned in the previous PR too, this is misleading and should be reworded more specifically like: "you can include only one model name in a batch call." what happens in the background of that request and whether the request gets routed to multiple models (classycat LLM vs classycat local classification) is not a concern for the consumer of the service, as long as it is getting out what it expected when issuing the call.
| * "top level" elements in the payload (outermost dictionary keys) are processed and controlled by the Presto system | ||
| * Elements inside parameters, and inside individual input items are passed through to the model | ||
| * From the perspective of the calling service, all the of the items in a single batch call must be dispatched to the same Presto model service. (don't mix items for paraphrase and classycat it the same call) | ||
| * Any parameter settings to the model must apply to all items in batch |
There was a problem hiding this comment.
My question here would be how would global parameters even be honored / used? Should all that info just get delegated down to the per-item level of detail? Could get messy to have that info at the task level
| } | ||
|
|
||
|
|
||
| Example PROPOSED input object structure (paraphrase-multilingual) |
There was a problem hiding this comment.
I think it's important to have a rough idea of what this looks like, since this is a very different model endpoint from classycat and yake.
| id:"11", | ||
| text:"this is some text to classify", | ||
| workspace_id:"timpani_meedan", | ||
| target_state:"classified", |
There was a problem hiding this comment.
do we currently have a model that can support multiple target states? otherwise I don't see how this is helpful and not something that would be a constant on every request.
There was a problem hiding this comment.
Yeah I'm thinking the same thing here - target_state seems implied by model_parameters. I think you either have it at the top-level or at the item-level but not both levels.
| ] | ||
| } | ||
|
|
||
| # Error response structures |
There was a problem hiding this comment.
it would be helpful if you could elaborate more on how we want to use the http error codes in responses. if an example helps, classycat returns a whole range of different error codes that would be good for explaining/reviewing a variety of use cases.
There was a problem hiding this comment.
Status codes make sense as top-level responses, sort of (they aren't actually responses, they are sending requests, so it is not technically a status code. I get if we want to use typical codes as loanwords for why something didn't work, but I can also see this getting strange over time since the receiving end isn't actually going to rescue them with HTTP exception logic). The issue I see with them is that they apply only to the top-level response but not on a per-item basis? How do we indicate per-item error, if at all?
| ] | ||
| } | ||
|
|
||
| # Error response structures |
There was a problem hiding this comment.
Status codes make sense as top-level responses, sort of (they aren't actually responses, they are sending requests, so it is not technically a status code. I get if we want to use typical codes as loanwords for why something didn't work, but I can also see this getting strange over time since the receiving end isn't actually going to rescue them with HTTP exception logic). The issue I see with them is that they apply only to the top-level response but not on a per-item basis? How do we indicate per-item error, if at all?
| id:"11", | ||
| text:"this is some text to classify", | ||
| workspace_id:"timpani_meedan", | ||
| target_state:"classified", |
There was a problem hiding this comment.
Yeah I'm thinking the same thing here - target_state seems implied by model_parameters. I think you either have it at the top-level or at the item-level but not both levels.
|
|
||
| General design | ||
|
|
||
| * Models that don't handle requests in parallel need to implement batch format (but can give errors if they recieve more than 1 item) |
There was a problem hiding this comment.
I think all models should support batch format, and those that can't do parallel ops just do them serially then return, IMO...
| * Models that don't handle requests in parallel need to implement batch format (but can give errors if they recieve more than 1 item) | ||
| * "top level" elements in the payload (outermost dictionary keys) are processed and controlled by the Presto system | ||
| * Elements inside parameters, and inside individual input items are passed through to the model | ||
| * From the perspective of the calling service, all the of the items in a single batch call must be dispatched to the same Presto model service. (don't mix items for paraphrase and classycat it the same call) |
There was a problem hiding this comment.
Hard agree on this assumption
| * "top level" elements in the payload (outermost dictionary keys) are processed and controlled by the Presto system | ||
| * Elements inside parameters, and inside individual input items are passed through to the model | ||
| * From the perspective of the calling service, all the of the items in a single batch call must be dispatched to the same Presto model service. (don't mix items for paraphrase and classycat it the same call) | ||
| * Any parameter settings to the model must apply to all items in batch |
There was a problem hiding this comment.
My question here would be how would global parameters even be honored / used? Should all that info just get delegated down to the per-item level of detail? Could get messy to have that info at the task level
| * Elements inside parameters, and inside individual input items are passed through to the model | ||
| * From the perspective of the calling service, all the of the items in a single batch call must be dispatched to the same Presto model service. (don't mix items for paraphrase and classycat it the same call) | ||
| * Any parameter settings to the model must apply to all items in batch | ||
| * Request itself needs a unique id, (and this is different than the id of the content) |
There was a problem hiding this comment.
Does it though? I could see not providing it and just getting a job id back that's a randomly assigned UUID on the response from presto?
Description
This splits off only the documentation of the presto batch refactor from #111. so that we can go ahead and merge it separately to close https://meedan.atlassian.net/browse/CV2-5117
Actually Implementing the refactor for yake and classycat will be another ticket