-
Notifications
You must be signed in to change notification settings - Fork 806
Concurrency limits for Nemotron PDF reader #1220
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds an optional concurrency parameter to the Nemotron PDF reader to control memory usage during page processing. Since all pages are currently processed concurrently, this can cause memory bursts. The new parameter allows users to limit concurrency on a page-by-page basis.
- Added
concurrencyparameter toparse_pdf_to_pagesfunction acceptingint | asyncio.Semaphore | None - Implemented conditional logic to use
gather_with_concurrencywhen concurrency limit is specified, otherwise useasyncio.gather - Enhanced
NemotronLengthErrorin the Nvidia API path to include response details for debugging
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/paper-qa-nemotron/src/paperqa_nemotron/reader.py | Added optional concurrency parameter to control concurrent page processing and prevent memory bursts |
| packages/paper-qa-nemotron/src/paperqa_nemotron/api.py | Enhanced NemotronLengthError exception to include response choice data for caller debugging |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
0d7e7c5 to
9b6732a
Compare
Since all pages are concurrently read, this is a bit bursty in memory usage. This PR adds an optional concurrency limit to limit the burst on a page-basis.