Improve BatchFeature (.to() works on lists/nested lists of tensors, auto-skip non array conversion) #42884
Conversation
|
[For maintainers] Suggested jobs to run (before merge) run-slow: glm46v, glm4v, llava_next_video, perception_lm, qwen2_5_omni, qwen2_5_vl, qwen2_vl, qwen3_omni_moe, qwen3_vl, video_llama_3 |
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
| # recursively handle lists and tuples | ||
| elif isinstance(v, (list, tuple)): | ||
| return type(v)(maybe_to(item) for item in v) |
There was a problem hiding this comment.
in which cases we have lists of tensors? I assume we either convert to 'pt' or skip and leave it as list
There was a problem hiding this comment.
A good example is in the issue I mentioned #42205
Basically for ragged lists of tensors
| return BatchFeature(data={**text_inputs, **image_inputs, **videos_inputs}, tensor_type=return_tensors) | ||
| return BatchFeature( | ||
| data={**text_inputs, **image_inputs, **videos_inputs}, | ||
| skip_tensor_conversion=["video_metadata"], |
There was a problem hiding this comment.
I think it can be added in as_tensor fn in batch features. We'll check if the input is array-like and do not convert if it's not. That also would generalize for similar outputs if any.
For ragged-arrays users will see and error because it's array-like structure and we don't auto-skip convertion. WDYT?
There was a problem hiding this comment.
Sounds good I'll see if we can do that in a robust way
…lly skip converting non array structure
yonigozlan
force-pushed
the
improve-batch-feature-further
branch
from
d9e2c83 to
686d9a3
Compare
yonigozlan
changed the title
zucchini-nlp
left a comment
zucchini-nlp
left a comment
There was a problem hiding this comment.
Thanks, lgtm! The chat template tests seems to be failing and skipping mask conversion to tensor, can you check?
Yep thanks for catching that! The issue was that I was skipping converting empty lists/nested lists when we shouldn't |
…uto-skip non array conversion) (huggingface#42884) * improve .to() to work on lists or nested lists of tensors, automatically skip converting non array structure * consider empty list or nested list as array-like
3 participants
What does this PR do?
Fixes #42205
Also fixes processors handling videos crashing when return_metadata=True and return_tensors="pt" .
Cc @zucchini-nlp , for now I have hardcoded all the skip conversions. I think it's safer to do this for now instead of silently not converting any inputs to BatchFeature for which tensor conversion fails. The main issue is with arrays, where non ragged arrays would be converted to tensors, and ragged arrays silently wouldn't, which could cause some unexpected behavior.
Open to alternative solutions so happy to hear your thoughts!
Edit: Added auto-skip, except for ragged arrays