[Bugfix] Fix lora tests#34834
Conversation
Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
|
This pull request has merge conflicts that must be resolved before it can be |
Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
There was a problem hiding this comment.
Code Review
This pull request addresses regressions in LoRA tests for multimodal models, specifically focusing on Qwen2-VL and Qwen2.5-VL. The changes refactor the multimodal LoRA resolution logic in the LLM entrypoint to be more robust and correctly handle individual requests within a batch. Additionally, the tests are updated to accommodate the full text output of beam search and fix assertion message formatting. I have identified one high-severity bug in a test assertion message that should be corrected to ensure clear failure reporting.
mgoin
left a comment
There was a problem hiding this comment.
LGTM just an assert message bug
Signed-off-by: Michael Goin <mgoin64@gmail.com>
|
Looks like LoRA 4 is still an issue |
|
Fixing |
alex-jw-brooks
left a comment
There was a problem hiding this comment.
Thanks for fixing these!
|
Resolved lora tests, other failures are unrelated |
Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk> Signed-off-by: Michael Goin <mgoin64@gmail.com> Co-authored-by: Michael Goin <mgoin64@gmail.com>
Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk> Signed-off-by: Michael Goin <mgoin64@gmail.com> Co-authored-by: Michael Goin <mgoin64@gmail.com>
Purpose
Regession was introduced by #34560 as
_resolve_lora_reqswasn't actually being called.FIX #34802
FIX #34804
Test Plan
Test Result
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.