Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideIntroduces a Sequence diagram for model loading with num_workers parametersequenceDiagram
participant User
participant API as audmodel.load()
participant Backend as get_archive()
User->>API: load(uid, cache_root, num_workers, verbose)
API->>Backend: get_archive(short_id, version, cache_root, num_workers, verbose)
Backend-->>API: returns archive path
API-->>User: returns archive path
Class diagram for updated load and get_archive functionsclassDiagram
class api {
+load(uid: str, cache_root: str | None = None, num_workers: int = 1, verbose: bool = False) str
}
class backend {
+get_archive(short_id: str, version: str, cache_root: str, num_workers: int, verbose: bool) str
}
api --> backend: calls get_archive
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `audmodel/core/api.py:277` </location>
<code_context>
uid: str,
*,
cache_root: str | None = None,
+ num_workers: int = 1,
verbose: bool = False,
) -> str:
</code_context>
<issue_to_address>
**suggestion:** Consider validating num_workers for positive integer values.
A check for num_workers > 0 will help prevent errors from invalid input.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| uid: str, | ||
| *, | ||
| cache_root: str | None = None, | ||
| num_workers: int = 1, |
There was a problem hiding this comment.
suggestion: Consider validating num_workers for positive integer values.
A check for num_workers > 0 will help prevent errors from invalid input.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files
🚀 New features to boost your workflow:
|
6b71aca to
5ea114e
Compare
Maybe we should extend |
Adds
num_workerstoaudmodel.load()to speed up model loading from backends that support downloads using multiple workers.This requires a development version of
audbackendandaudeerat the moment.Benchmarks
All benchmarks use the model
7289b57d-1.0.0(4.2 GB).I run the benchmark with
benchmark-audmodel.py
Performance when using the single-threaded implementations of
audeerandaudbackend:When using only multiple workers with
audbackend, performance gets only slightly better. So most likely the archive extraction is costly.When using multiple workers with
audbackendandaudeer, performance only slightly gets better. I think the biggest problem is that we store the model in a single big file. Using multiple workers during extraction of the archive is only efficient if the model would be stored in several smaller chunks.Summary by Sourcery
Introduce a num_workers parameter to the model loading API and backend to enable parallel downloads, and update the audbackend dependency to a development branch that supports multi-worker downloads.
New Features:
Build: