Skip to content

Conversation

@edknv
Copy link
Collaborator

@edknv edknv commented Apr 2, 2025

Description

This PR moves from llama_index.embeddings.nvidia import NVIDIAEmbedding inside the nvingest_retrieval() function in order to prevent top-level import from triggering logging:

[nltk_data] Downloading package punkt_tab to
[nltk_data]     /opt/conda/envs/nv_ingest_runtime/lib/python3.10/site-
[nltk_data]     packages/llama_index/core/_static/nltk_cache...
[nltk_data]   Unzipping tokenizers/punkt_tab.zip.

and downloading when the module is not used.

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.
  • If adjusting docker-compose.yaml environment variables have you ensured those are mimicked in the Helm values.yaml file.

@edknv edknv requested a review from a team as a code owner April 2, 2025 17:21
@edknv edknv requested review from drobison00 and jperez999 and removed request for a team April 2, 2025 17:21
Copy link
Collaborator

@jperez999 jperez999 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hopefully the import time does not cause a problem (notable decrease in qps) for a user who is trying to use the nvingest_retrieval function in a loop over lots of requests. Until we get a complaint, I am fine with this move.

@jperez999 jperez999 merged commit d649d90 into NVIDIA:main Apr 2, 2025
4 of 5 checks passed
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.

2 participants