Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Long context for DocSum #981

Merged
merged 22 commits into from
Dec 17, 2024
Merged

Conversation

XinyaoWa
Copy link
Collaborator

@XinyaoWa XinyaoWa commented Dec 6, 2024

Description

Support Long context for DocSum with four modes:
• Stuff (Default mode): input actual tokens, need to increase the max_input_token if want to use large context
• Truncate: truncate the tokens exceed the limitation
• Map_reduce: split the inputs into multiple chunks, map each document to an individual summary, then consolidate those summaries into a single global summary
• Refine: split the inputs into multiple chunks, generate summary for the first one, then combine with the second, loops over every remaining chunks to get the final summary

Issues

List the issue or RFC link this PR is working on. If there is no such link, please mark it as n/a.

Type of change

List the type of change like below. Please delete options that are not relevant.

  • New feature (non-breaking change which adds new functionality)

Dependencies

List the newly introduced 3rd party dependency if exists.

Tests

Describe the tests that you ran to verify your changes.

Signed-off-by: Xinyao Wang <[email protected]>
Signed-off-by: Xinyao Wang <[email protected]>
Signed-off-by: Xinyao Wang <[email protected]>
Signed-off-by: Xinyao Wang <[email protected]>
Copy link

codecov bot commented Dec 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Files with missing lines Coverage Δ
comps/cores/proto/docarray.py 99.44% <100.00%> (+0.01%) ⬆️

... and 1 file with indirect coverage changes

@eero-t
Copy link
Contributor

eero-t commented Dec 12, 2024

docsum wrapper for TGI failed in CI with: ImportError: cannot import name 'DocSumLLMParams' from 'comps' (/home/user/comps/__init__.py)

@jjmaturino
Copy link

@eero-t How long does it usually take for these PR's to get reviewed? I opened a couple this week...

@eero-t
Copy link
Contributor

eero-t commented Dec 12, 2024

@eero-t How long does it usually take for these PR's to get reviewed? I opened a couple this week...

I'm not developer in this project, but people assume PR to be incomplete / not ready for review when it fails CI tests.

(If you think CI failure is not due to your own changes, file ticket for it, and state that it blocks the PR.)

Once CI tests pass, I think you could ping here one of the people who've reviewed / merged other PRs, if nobody comments it within few days / weeks (depending on how urgent you think it is). As you're from Intel, you could also ping OPEA Intel devs internally.

Signed-off-by: Xinyao Wang <[email protected]>
@XinyaoWa
Copy link
Collaborator Author

@eero-t How long does it usually take for these PR's to get reviewed? I opened a couple this week...

Hi, sorry I was busy with other projects some time ago, now I will continue to fix this PR, it should be ready in 1~2 workdays!

@eero-t
Copy link
Contributor

eero-t commented Dec 13, 2024

I do not see why example-test is failing, but the llms_summarization_tgi_langchain, intel_cpu test issue seems like some (hopefully temporary) issue with CI, as container it creates from image is not available:

+ docker run -d --name=test-comps-llm-sum-tgi-server -p 5076:9000 --ipc=host -e http_proxy= -e https_proxy= -e TGI_LLM_ENDPOINT=http://100.80.243.74:5075/ -e HUGGINGFACEHUB_API_TOKEN=*** opea/llm-sum-tgi:comps
6e2f7a14694b36b68359f0764a6b7bf12a0a754d244d82baa60e415f42a13a19
docker: Error response from daemon: container is marked for removal and cannot be started.
...
Error response from daemon: No such container: test-comps-llm-sum-tgi-server
+ exit 1
Error: Process completed with exit code 1.

@XinyaoWa XinyaoWa changed the title [WIP] Support Long context for DocSum Support Long context for DocSum Dec 16, 2024
@eero-t
Copy link
Contributor

eero-t commented Dec 16, 2024

Current CI failure is due to bad SERVICE_NAME value:

+ local 'SERVICE_NAME=llm - summarization'
...
./llms/test_llms_summarization_tgi_langchain.sh: line 71: ${LOG_PATH}/${SERVICE_NAME}.log: ambiguous redirect

Please use shellcheck (from shellcheck package) to check that scripts handle things like arguments properly.

Signed-off-by: Xinyao Wang <[email protected]>
Signed-off-by: Xinyao Wang <[email protected]>
Signed-off-by: Xinyao Wang <[email protected]>
@XinyaoWa XinyaoWa merged commit 5aba3b2 into opea-project:main Dec 17, 2024
17 checks passed
smguggen pushed a commit to opea-aws-proserve/GenAIComps that referenced this pull request Jan 23, 2025
* docsum four

Signed-off-by: Xinyao Wang <[email protected]>

* support 4 modes for docsum

Signed-off-by: Xinyao Wang <[email protected]>

* fix

Signed-off-by: Xinyao Wang <[email protected]>

* fix bug

Signed-off-by: Xinyao Wang <[email protected]>

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* refine for docsum tgi

Signed-off-by: Xinyao Wang <[email protected]>

* add docsum for ut and vllm

Signed-off-by: Xinyao Wang <[email protected]>

* fix bug

Signed-off-by: Xinyao Wang <[email protected]>

* fix bug

Signed-off-by: Xinyao Wang <[email protected]>

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* fix ut bug

Signed-off-by: Xinyao Wang <[email protected]>

* fix ut bug

Signed-off-by: Xinyao Wang <[email protected]>

* set default value

Signed-off-by: Xinyao Wang <[email protected]>

---------

Signed-off-by: Xinyao Wang <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants