Repository navigation
🚨Fix floating-point precision in JanusImageProcessor resize - #43187
Conversation
|
Bug seems legit but I'd like someone with more image processor experience to check that we're still matching the behaviour that these models expect, because sometimes models are trained with slightly buggy preprocessing code and it becomes important to match that behaviour later. cc @molbap @yonigozlan @zucchini-nlp |
zucchini-nlp
left a comment
There was a problem hiding this comment.
Makes sense to me, we need the sides to be exactly 'size' length. Can you run make fix-repo to convert modular files?
|
Done. Ran the modular converter and pushed the regenerated file. Note: The same int() truncation issue exists in two other places:
Should I include these in this PR or handle them separately? |
Running |
|
Fixed. Added the missing changes for deepseek_vl and janus_fast. |
|
@bot /repo |
|
Repo. Consistency fix runs successfully without any file modified. |
|
run-slow: deepseek_vl, janus, deepseek_vl_hybrid |
|
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. |
|
run-slow: deepseek_vl, janus, deepseek_vl_hybrid |
|
slow tests cannot be triggered 😢 |
|
run-slow: deepseek_vl, deepseek_vl_hybrid, janus |
|
This comment contains models: ["models/deepseek_vl", "models/deepseek_vl_hybrid", "models/janus"] |
CI ResultsModel CI Report❌ Failed tests
|
|
I got similar results locally with slow tests now failing due to this changes. However it seems to me that the processing now works as it's supposed to , so ok to change the expected values (and add a 🚨 as it breaks BC a bit)? @molbap @zucchini-nlp |
|
Yeah, sounds good to me. Let's just update test values with the new ones and merge as breaking |
|
run-slow: deepseek_vl, deepseek_vl_hybrid, janus |
|
[For maintainers] Suggested jobs to run (before merge) run-slow: deepseek_vl, deepseek_vl_hybrid, janus |
Fixes #43186
What does this PR do?
Fixes floating-point precision issue in
JanusImageProcessor.resize()whereint()truncation caused incorrect output dimensions.Problem:
Solution:
Replace
int()withround()for proper rounding of scaled dimensions.Why edit
modular_janus.py?The file
image_processing_deepseek_vl.py(mentioned in the issue) is auto-generated frommodular_janus.py. The header states:DeepseekVL imports
JanusImageProcessor, so fixing the source fixes all dependent models: