Repository navigation
Processing tests [part 2] - #47922
Conversation
|
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. |
| def __call__(self, text=None, images=None, visual_prompt=None, return_tensors=None, **kwargs): | ||
| def __call__(self, text=None, images=None, visual_prompt=None, **kwargs): | ||
| r""" |
There was a problem hiding this comment.
why separate, should be consumed via kwargs and merged in _merge_kwargs
|
Also should be ready now for a second review |
vasqu
left a comment
vasqu
left a comment
There was a problem hiding this comment.
Overall happy with the state of the PR, just a few smaller comments but it's nothing major imo
| special_token_to_add = "" | ||
| if modalities is not None: | ||
| for modality in modalities: | ||
| # we have non-uniform naming conventions for image/videos |
There was a problem hiding this comment.
Follow up PR at some point? Just thinking for myself tbh
There was a problem hiding this comment.
i really dont want to, we will be dragging a huge deprecation cycle for a really long time. maybe for v6 after another 4 years 😆
| return [prepare_image_inputs()] * batch_size | ||
| return prepare_image_inputs() * batch_size | ||
| return [prepare_images_inputs()] * batch_size | ||
| return prepare_images_inputs() * batch_size |
There was a problem hiding this comment.
Ok maybe some ocd kicking in me but we use modality="image" and name ourselves with plural "prepare_images"
There was a problem hiding this comment.
every place uses plural so in this PR i am trying to converge to it. In token IDs it is firstly BC, secondly weird/inconsistent with config classes
Most breaking point is public call API for all types of processors which always has been plural 😢 And everythgin after that copied plural form which is tbh not bad per se
There was a problem hiding this comment.
Yea it's not really, just gotta be consistent with the naming at some point :D
| return_dict=True, | ||
| return_tensors=return_tensors, | ||
| processor_kwargs={"num_frames": 2}, # by default no more than 2 frames, otherwise too slow | ||
| processor_kwargs={"num_frames": 2, "fps": None}, # no more than 2 frames, otherwise too slow |
|
I will merge it tmrw if fine and ask audio team to add audio tests. We'll probably need a |
|
Sounds good, yes no worries. This PR is already big enough as is, let's unbloat step by step My fear is the audio inconcistency but we will figure it out |
|
Why 😢 Update: the new error after rebase is valid, checking |
|
[For maintainers] Suggested jobs to run (before merge) run-slow: aria, audioflamingo3, aya_vision, bridgetower, chameleon, clipseg, cohere_compass, colmodernvbert, colpali, colqwen2, cosmos3_edge, csm, deepseek_ocr2, emu3, ernie4_5_vl_moe, evolla |
CI recapDashboard: View test results in Grafana |
* . * draft it * fix some * maybe * . * chat template in qwen style * draft sth for video sampling tests * fix * . * fix last * move around * draft * . * renamed those * fix repo deleted one line in all files * fix * the rest * merge this * typo * when will this end? * fix? * save coz now i will break stuff * rename it all, except for xx_token_id * missed these renames * fix last * revert * comments * less small private helpers * more comments * last failing tests, and i stop adding more stuff * typo * forgot about this one * why i cannot get green CI * move around to skip more tests * few more * fix * new model * here it is, opened PR on the hub * forgot modular...
What does this PR do?
return_replacement_offsetswhich will be used by vllm and finally become the only preferred way overmm_token_type_idsas it enables processing cache for simple modelstest_chat_templatefor videos by allowing each child to define only input sampling args and expected number of samples frames