Skip to content

Conversation

smirnov-alexey
Copy link
Contributor

No description provided.

@smirnov-alexey smirnov-alexey requested review from a team as code owners August 26, 2025 09:55
@github-actions github-actions bot added the category: NPU OpenVINO NPU plugin label Aug 26, 2025
Copy link
Contributor

@dmatveev dmatveev left a comment

Choose a reason for hiding this comment

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

If it fixes some behavior, I believe there must be some tests reflecting it?

DEFINE_OPT(NPUW_LLM_CACHE_ROPE, bool, true, npuw::llm::cache_rope, CompileTime);
DEFINE_OPT(NPUW_LLM_CACHE_ROPE, bool, true, npuw::llm::cache_rope, RunTime);
Copy link
Contributor

Choose a reason for hiding this comment

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

It impacts the IR that's being compiled, how it is a runtime option?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All of our properties are RunTime. CompileTime is used for the NPU compiler. Also caching property won't work if it's not RunTime since we would to define isAvailable in filteredConfig somewhere in the plugin

DEFINE_OPT(NPUW_LLM_SHARED_HEAD, bool, true, npuw::llm::shared_lm_head, CompileTime);
DEFINE_OPT(NPUW_LLM_SHARED_HEAD, bool, true, npuw::llm::shared_lm_head, RunTime);
Copy link
Contributor

Choose a reason for hiding this comment

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

Same thing here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Answered above

Comment on lines 250 to 251
struct NPUW_LLM_ADDITIONAL_PREFILL_CONFIG final : OptionBase<NPUW_LLM_ADDITIONAL_PREFILL_CONFIG, ov::AnyMap> {
static std::string_view key() {
Copy link
Contributor

Choose a reason for hiding this comment

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

What is this, why

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To support caching for any additional configs we might pass

@@ -276,4 +306,95 @@ struct NPUW_LLM_GENERATE_CONFIG final : OptionBase<NPUW_LLM_GENERATE_CONFIG, ov:
return false;
}
};

struct NPUW_LLM_ADDITIONAL_GENERATE_CONFIG final : OptionBase<NPUW_LLM_ADDITIONAL_GENERATE_CONFIG, ov::AnyMap> {
Copy link
Contributor Author

@smirnov-alexey smirnov-alexey Aug 26, 2025

Choose a reason for hiding this comment

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

Move under macro

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@smirnov-alexey
Copy link
Contributor Author

build_jenkins

Copy link
Contributor

@dmatveev dmatveev left a comment

Choose a reason for hiding this comment

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

Thanks to the refactoring the net effect is just two more lines - that's the way to go!

@smirnov-alexey
Copy link
Contributor Author

build_jenkins

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: NPU OpenVINO NPU plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants