Skip to content

preserve properties from allowlist in Tokenizer #2604

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Aug 13, 2025
Merged

preserve properties from allowlist in Tokenizer #2604

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
42e0f6f
preserve properties from white list in Tokenizer
pavel-esir Aug 13, 2025
dd9430e
Update src/cpp/src/tokenizer/tokenizer.cpp
pavel-esir Aug 13, 2025
49c5c42
update win cache
pavel-esir Aug 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 26 additions & 3 deletions src/cpp/src/tokenizer/tokenizer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,29 @@ class Tokenizer::TokenizerImpl {
setup_tokenizer(models, properties);
}

void filter_properties(ov::AnyMap& properties) {
// Properties allowed for tokenizer/detokenizer on CPU
std::set<std::string> allowed_argnames = {
ov::hint::performance_mode.name(),
ov::hint::num_requests.name(),
ov::hint::enable_cpu_pinning.name(),
ov::hint::execution_mode.name(),
ov::hint::compiled_blob.name(),
ov::hint::enable_hyper_threading.name(),
ov::hint::enable_cpu_reservation.name(),
ov::enable_profiling.name(),
};

for (auto prop_it = properties.begin(); prop_it != properties.end();) {
auto it = allowed_argnames.find(prop_it->first);
if (it == allowed_argnames.end()) {
prop_it = properties.erase(prop_it);
} else {
++prop_it;
}
}
}

void setup_tokenizer(const std::filesystem::path& models_path, const ov::AnyMap& properties) {
ScopedVar env_manager(tokenizers_relative_to_genai());
auto core = get_core_singleton();
Expand Down Expand Up @@ -369,9 +392,9 @@ class Tokenizer::TokenizerImpl {
two_input_requested = it->second.as<bool>();
properties.erase(it);
}

// Pass no addtional properties to tokenizer/detokenizer models since it was not used by default
properties = {};
// Filter properties by leaving only params from the allowlist
filter_properties(properties);
Copy link
Collaborator

@Wovchena Wovchena Aug 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This overload doesn't have a path so enable_save_ov_model should result in failure. I think you shouldn't filter properties

Copy link
Collaborator

Choose a reason for hiding this comment

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

You shouldn't filter*

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we don't filter, and allow all properties to pass to 'CPU' then it fails because cpu plugin

2025-08-12T13:35:28.4948146Z E       Exception from src/inference/src/dev/plugin.cpp:53:
2025-08-12T13:35:28.4948893Z E       Exception from src/plugins/intel_cpu/src/config.cpp:457:
2025-08-12T13:35:28.4949480Z E       NotFound: Unsupported property NPUW_DEVICES by CPU plugin.
...
2025-08-12T13:35:28.4924214Z E           Exception from src/plugins/intel_cpu/src/config.cpp:457:
2025-08-12T13:35:28.4924878Z E           NotFound: Unsupported property prompt_lookup by CPU plugin.
...

and so on. We should leave only properties which core.compile_model(ov_[de]tokenizer, properties) can handle.

I also didn't quite understand bout saved model. At this point regardless of whether i filter or we don't have 'enable_save_ov_model' in properties. Could you please elaborate what did you mean?

Copy link
Collaborator

Choose a reason for hiding this comment

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

If there's no enable_save_ov_model, what property leads to the exception?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At least 3 arguments above NPUW_DEVICES, prompt_lookup, draft_model, probably some more

Copy link
Collaborator

@Wovchena Wovchena Aug 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I guess that should do for now. But this is not a full list of valid properties. It will need to be converted to a black list instead of white list in the next PR because it's easier to resolve a problem reported as an error rather then just observe perf degradation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree with you


is_paired_input = ov_tokenizer && ov_tokenizer->get_parameters().size() == 2;

Expand Down
Loading