jina-ai / serve

☁️ Build multimodal AI applications with cloud-native stack
https://jina.ai/serve
Apache License 2.0
21.14k stars 2.22k forks source link

fix: batch transform update for sagemaker reranker integration #6145

Closed zac-li closed 9 months ago

zac-li commented 9 months ago

Goals: This PR made a required improvement (based off of https://github.com/jina-ai/jina/pull/6055/) on the SageMaker Batch Transform routine so that Reranking SageMaker model can perform Batch Transform as expected.

Previously, Jina Batch Transform endpoint has no problem reading simple data in each row from CSV and construct a input model and perform inference, for example, for

0f37fc919deb46e8bcafd92f1feba65f, simple text, it will be converted to:

TextDoc(
    id='0f37fc919deb46e8bcafd92f1feba65f',
    text='simple text'
)

However, for more complex model (e.g., includes List of models), such as:

class RankInput(OriginalRankInput):
    query: Union[str, TextDoc]
    documents: List[TextDoc]
    top_n: Optional[int]

which is represented as such in CSV:

07937cd63aed436b9e6c34a86783f55f,where's the dog,[{\"text\": \"the dog is in my house\"}\, {\"text\": \"the cat looks good\"}\, {\"text\": \"fish chips\"}],2

The original implementation would fail when parsing.

So this PR proposed a construct_model_from_line that can deal with more complex cases like above.

Test cases have been added to verify the fix.

codecov[bot] commented 9 months ago

Codecov Report

Attention: Patch coverage is 0% with 35 lines in your changes are missing coverage. Please review.

Project coverage is 75.04%. Comparing base (af406ce) to head (8577b09). Report is 2 commits behind head on master.

Files Patch % Lines
jina/serve/runtimes/worker/http_sagemaker_app.py 0.00% 31 Missing :warning:
jina/serve/executors/__init__.py 0.00% 4 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #6145 +/- ## ========================================== - Coverage 75.22% 75.04% -0.19% ========================================== Files 152 152 Lines 14062 14095 +33 ========================================== - Hits 10578 10577 -1 - Misses 3484 3518 +34 ``` | [Flag](https://app.codecov.io/gh/jina-ai/jina/pull/6145/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=jina-ai) | Coverage Δ | | |---|---|---| | [jina](https://app.codecov.io/gh/jina-ai/jina/pull/6145/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=jina-ai) | `75.04% <0.00%> (-0.19%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=jina-ai#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

JoanFM commented 9 months ago

tests are failing