Skip to content

Remove stale top_k workaround (HF bug fixed upstream)#497

Merged
satra merged 2 commits into
alphafrom
fix/remove-top-k-workaround
Apr 30, 2026
Merged

Remove stale top_k workaround (HF bug fixed upstream)#497
satra merged 2 commits into
alphafrom
fix/remove-top-k-workaround

Conversation

@satra

@satra satra commented Apr 30, 2026

Copy link
Copy Markdown
Collaborator

transformers#35736 fixed Feb 2025. Pass top_k directly to pipeline constructor.

The transformers#35736 bug (top_k in pipeline constructor) was fixed
in Feb 2025. Remove the workaround comment and pass top_k directly
to pipeline() constructor as intended.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request re-enables the 'top_k' and 'function_to_apply' parameters in the Hugging Face audio classification pipeline. Feedback suggests also passing the 'batch_size' parameter to the pipeline constructor to ensure consistency with the cache key and improve performance. Additionally, a potential TypeError was identified when 'top_k' is set to 1, as the pipeline's return format changes from a list to a single dictionary, which may break post-processing logic.

Comment on lines +55 to +56
top_k=top_k,
function_to_apply=function_to_apply,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

The batch_size parameter is included in the cache key but is not passed to the pipeline constructor, which means the pipeline will default to a batch size of 1, ignoring the user's preference and potentially impacting performance.

Additionally, please note that when top_k=1, the Hugging Face pipeline returns a single dictionary per input instead of a list of dictionaries. This will cause a TypeError in the post-processing logic at line 158, which expects a list of dictionaries to iterate over.

Suggested change
top_k=top_k,
function_to_apply=function_to_apply,
top_k=top_k,
function_to_apply=function_to_apply,
batch_size=batch_size,

batch_size is passed at pipe() call time, not pipeline() constructor.
Including it in the cache key created unnecessary duplicate pipelines.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@satra
satra merged commit 6559f0c into alpha Apr 30, 2026
9 checks passed
@satra
satra deleted the fix/remove-top-k-workaround branch April 30, 2026 18:19
github-actions Bot added a commit that referenced this pull request Apr 30, 2026
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.

1 participant