-
Notifications
You must be signed in to change notification settings - Fork 284
Safe VLM JSON config parsing with read_json_param()
#2785
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Safe VLM JSON config parsing with read_json_param()
#2785
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR addresses a JSON config parsing issue in the VLM (Visual Language Model) processing where transformers
library v4.55+ exports configs with null
values that cause runtime errors. The solution replaces manual JSON key checking with the safer read_json_param()
utility function across VLM configuration parsing.
- Replaced manual
contains()
checks and directat()
access withread_json_param()
calls - Added null-safety checks for array parameters in phi3_v and phi4mm models
- Enhanced
read_json_param()
to support nested key paths andstd::array
types
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
src/cpp/src/visual_language/vlm_config.cpp | Updated VLM config parsing to use safe read_json_param() with nested keys and added array type checks |
src/cpp/src/visual_language/processor_config.cpp | Replaced all manual JSON parsing with read_json_param() calls for safer config loading |
src/cpp/src/json_utils.hpp | Added std::array type trait and support for array types in read_json_param() |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
if (data[name].is_number() || data[name].is_boolean() || data[name].is_string() || data[name].is_object() | ||
|| (is_std_array<T> && data[name].is_array()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The condition logic could be simplified and made more readable by extracting the array condition into a separate variable or reorganizing the boolean expressions for better clarity.
if (data[name].is_number() || data[name].is_boolean() || data[name].is_string() || data[name].is_object() | |
|| (is_std_array<T> && data[name].is_array()) | |
bool is_compatible_array = is_std_array<T> && data[name].is_array(); | |
if (data[name].is_number() || data[name].is_boolean() || data[name].is_string() || data[name].is_object() | |
|| is_compatible_array |
Copilot uses AI. Check for mistakes.
5ee23bb
Description
transformers
library starting from version 4.55 savespreprocessor_config.json
a bit differently during model exporting withoptimum-intel
- they add some fields withnull
values (e.g. "crop_size" inQwen/Qwen2.5-VL-3B-Instruct
case).GenAI is checking if the config contains specific keys (depending on the model), and if so it extracts values by the key or nested keys. Having params in json config with
null
values causing the errorRuntimeError: [json.exception.type_error.304] cannot use at() with null
.This PR provides safe json config parsing by using
read_json_param()
utility function.Ticket: 174249
Checklist: