Skip to content

[Fix] Swapped height and width in KimiK25 - #47786

Merged
remi-or merged 4 commits into
mainfrom
fix-kimi25-hw
Aug 5, 2026
Merged

[Fix] Swapped height and width in KimiK25#47786
remi-or merged 4 commits into
mainfrom
fix-kimi25-hw

Conversation

@remi-or

@remi-or remi-or commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

CI

This PR fixes an height / width inversion that was impacting Kimi K2.5 : navit_resize was called with the height and width arguments swapped. This is fixed in this PR and a regression test was added.
Also, the navit_resize function was duplicated across two files (image and video processing) so now video only import from imagr rather than re-writing the fn.

@remi-or
remi-or requested a review from zucchini-nlp August 5, 2026 05:13
@remi-or remi-or self-assigned this Aug 5, 2026
@remi-or

remi-or commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator Author

run-slow: kimi_k25

@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Workflow Run ⚙️

This comment contains run-slow, running the specified jobs:

models: ["models/kimi_k25"]
quantizations: []

@HuggingFaceDocBuilderDev

Copy link
Copy Markdown

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.

@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

CI Results

Workflow Run ⚙️

Commit Info

Context Commit Description
RUN 12a74aee workflow commit (merge commit)
PR 6be3a8a8 branch commit (from PR)
main d24d79da base commit (on main)

✅ No failing test specific to this PR 🎉 👏 !

@zucchini-nlp zucchini-nlp left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks, let's just revert the cross-file import :)

Comment on lines -36 to -39
# Same resize as in image processing
def navit_resize(
width: int,
height: int,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this needs to be defined, as requested by core maintainers :)
recently we updated qwen-format processors to swap import with explicit redefinition, see #47573 (comment)

@remi-or remi-or Aug 5, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Ok no problem! Just for my understanding sorry: why do we not like cross imports?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

bit of backstory - we had import issues in the past when importing between PIL-Torch backends, then Arthur decided to explicitly define these fn. Later on we also noticed modular doesn't work well with it, so now we also redefine them in video processors

and i guess it goes against "our philosophy of one file"?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

(tbh i also don't like redefining it 😆 )

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Ok thanks for the info!

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Mostly modular tbh, otherwise if we only implement the img or vid processor then we inherit the cross import and some magic later we try to import from non existing files

@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

[For maintainers] Suggested jobs to run (before merge)

run-slow: kimi_k25

@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

CI recap

Dashboard: View test results in Grafana
Latest run: 30985250588:1
Result: success | Jobs: 3 | Tests: 66 | Failures: 0 | Duration: 4s

@remi-or
remi-or added this pull request to the merge queue Aug 5, 2026
Merged via the queue into main with commit 5d6aa36 Aug 5, 2026
33 checks passed
@remi-or
remi-or deleted the fix-kimi25-hw branch August 5, 2026 09:06
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.

4 participants