WWB Text Generation with LoRA #2723

likholat · 2025-09-11T14:41:11Z

Copilot

Pull Request Overview

This PR adds LoRA (Low-Rank Adaptation) support to the Who What Benchmark text generation pipeline, addressing issues CVS-173476 and CVS-173478. The changes enable loading and using LoRA adapters with both OpenVINO GenAI and HuggingFace Transformers backends.

Added LoRA adapter configuration support for OpenVINO GenAI and HuggingFace pipelines
Enhanced text generation functions to handle empty adapter configurations
Updated help text to clarify LoRA adapter support for Text Generation and Image Generation pipelines

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
tools/who_what_benchmark/whowhatbench/wwb.py	Updated help text and enhanced genai_gen_text function with empty_adapters parameter
tools/who_what_benchmark/whowhatbench/text_evaluator.py	Added empty_adapters parameter support to TextEvaluator class
tools/who_what_benchmark/whowhatbench/model_loaders.py	Implemented LoRA adapter loading for both OpenVINO GenAI and HuggingFace pipelines

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-09-11T15:05:55Z

tools/who_what_benchmark/whowhatbench/model_loaders.py

        exit(-1)

+    adapter_config = openvino_genai.AdapterConfig()
+    if "adapters" in kwargs and kwargs["adapters"] is not None:


Potential KeyError if 'alphas' is not present in kwargs. The code assumes kwargs['alphas'] exists when adapters are provided, but there's no validation to ensure this key exists.

Suggested change

if "adapters" in kwargs and kwargs["adapters"] is not None:

if "adapters" in kwargs and kwargs["adapters"] is not None:

if "alphas" not in kwargs or kwargs["alphas"] is None:

raise ValueError("If 'adapters' are provided, 'alphas' must also be provided in kwargs.")

Copilot · 2025-09-11T15:05:56Z

tools/who_what_benchmark/whowhatbench/model_loaders.py

+            assert len(alphas) == len(adapter_names), "`alphas` must be the same length as `adapters`"
+            model.add_weighted_adapter([f"adapter_{idx}" for idx in range(len(kwargs['adapters']))], alphas, "merged_lora")
+        else:
+            model.add_weighted_adapter([f"adapter_{idx}" for idx in range(len(kwargs['adapters']))], "merged_lora")


Missing weights parameter in add_weighted_adapter call. The method expects weights as the second parameter, but 'merged_lora' is passed as the second parameter instead of the adapter_name parameter.

Suggested change

model.add_weighted_adapter([f"adapter_{idx}" for idx in range(len(kwargs['adapters']))], "merged_lora")

# If no alphas provided, use equal weights

model.add_weighted_adapter(

[f"adapter_{idx}" for idx in range(len(kwargs['adapters']))],

[1.0] * len(kwargs['adapters']),

"merged_lora"

)

sbalandi · 2025-09-12T09:43:34Z

tools/who_what_benchmark/whowhatbench/model_loaders.py

                model_id, trust_remote_code=True, device_map=device.lower(), **model_kwargs
            )

+    if 'adapters' in kwargs and kwargs['adapters'] is not None:


Suggested change

if 'adapters' in kwargs and kwargs['adapters'] is not None:

if kwargs.get("adapters") is not None:

sbalandi · 2025-09-12T09:46:59Z

tools/who_what_benchmark/whowhatbench/model_loaders.py

+
+        if alphas is not None:
+            assert len(alphas) == len(adapter_names), "`alphas` must be the same length as `adapters`"
+            model.add_weighted_adapter([f"adapter_{idx}" for idx in range(len(kwargs['adapters']))], alphas, "merged_lora")


Isn't this [f"adapter_{idx}" for idx in range(len(kwargs['adapters']))] the same as adapter_names ?

sbalandi · 2025-09-12T09:49:33Z

tools/who_what_benchmark/whowhatbench/model_loaders.py

        logger.info("Using OpenVINO GenAI Continuous Batching API")
        scheduler_config = get_scheduler_config_genai(kwargs["cb_config"])
-        pipeline = openvino_genai.LLMPipeline(model_dir, device=device, scheduler_config=scheduler_config, **ov_config)
+        pipeline = openvino_genai.LLMPipeline(model_dir, device=device, adapters=adapter_config, scheduler_config=scheduler_config, **ov_config)


Are behavior with:

adapter_config = openvino_genai.AdapterConfig() openvino_genai.LLMPipeline(model_dir, device=device, adapters=adapter_config, **ov_config)

and
openvino_genai.LLMPipeline(model_dir, device=device, **ov_config)
the same ?

Yes, it is, I'll also add a separate check for the validation team on this

sbalandi · 2025-09-12T09:54:46Z

tools/who_what_benchmark/whowhatbench/model_loaders.py


+    adapter_config = openvino_genai.AdapterConfig()
+    if "adapters" in kwargs and kwargs["adapters"] is not None:
+        for adapter, alpha in zip(kwargs['adapters'], kwargs['alphas']):


If the alpha value does not match the number of adapters, then here it will be processed and adapters and alphas will be combined, but for hf it will fail. Why ? Is it impossible for hf to combine adapters with and without alphas?

I’d just add an assert here too: if the number of adapters and alphas don’t match, it’s unclear which alpha applies to which adapter, most likely the user forgot to provide a weight

sbalandi · 2025-09-12T15:17:07Z

tools/who_what_benchmark/whowhatbench/model_loaders.py

        exit(-1)

+    adapter_config = openvino_genai.AdapterConfig()
+    if "adapters" in kwargs and kwargs["adapters"] is not None:


Suggested change

if "adapters" in kwargs and kwargs["adapters"] is not None:

if kwargs.get("adapters") is not None:

sbalandi · 2025-09-12T15:24:14Z

tools/who_what_benchmark/whowhatbench/model_loaders.py


+    adapter_config = openvino_genai.AdapterConfig()
+    if "adapters" in kwargs and kwargs["adapters"] is not None:
+        assert len(kwargs['alphas']) == len(kwargs["adapters"]), "`alphas` must be the same length as `adapters`"


Can adapter be set without alpha for genAI ? if yes, this case needs to be handled as well

Suggested change

assert len(kwargs['alphas']) == len(kwargs["adapters"]), "`alphas` must be the same length as `adapters`"

if kwargs.get('alphas') is not None:

assert len(kwargs['alphas']) == len(kwargs["adapters"]), "`alphas` must be the same length as `adapters`"

for adapter, alpha in zip(kwargs['adapters'], kwargs['alphas']):

....

or if adapters can't be set without alphas

Suggested change

assert len(kwargs['alphas']) == len(kwargs["adapters"]), "`alphas` must be the same length as `adapters`"

assert len(kwargs['alphas'], []) == len(kwargs["adapters"]), "`alphas` must be the same length as `adapters`"

Adapter can be set without alpha for all the cases (genai, optimum, hf).
I don’t really understand the purpose of the suggested change - it already works correctly when alpha is not provided

if alphas is not provided len(kwargs['alphas']) will be mean len(None) and it should rise TypeError. This doesn't happen now?

you are right, I have found the place , alphas are converted to list and it never will be None

Then, this check assert len(kwargs['alphas']) == len(kwargs["adapters"]), "alphas must be the same length as adapters" can be moved to https://github.com/openvinotoolkit/openvino.genai/blob/master/tools/who_what_benchmark/whowhatbench/wwb.py#L592 and it doesn't need to keep

openvino.genai/tools/who_what_benchmark/whowhatbench/model_loaders.py

Line 145 in 68c20f8

model.add_weighted_adapter(adapter_names, "merged_lora")

since this code will never be executed

Copilot

Pull Request Overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-09-15T09:31:30Z

tools/who_what_benchmark/whowhatbench/model_loaders.py

+    if kwargs.get("adapters") is not None:
+        assert len(kwargs['alphas']) == len(kwargs["adapters"]), "`alphas` must be the same length as `adapters`"
+        for adapter, alpha in zip(kwargs['adapters'], kwargs['alphas']):


This assertion will fail when alphas is None but adapters is provided. The code should check if alphas exists before validating lengths, or handle the None case gracefully.

Suggested change

if kwargs.get("adapters") is not None:

assert len(kwargs['alphas']) == len(kwargs["adapters"]), "`alphas` must be the same length as `adapters`"

for adapter, alpha in zip(kwargs['adapters'], kwargs['alphas']):

adapters = kwargs.get("adapters")

alphas = kwargs.get("alphas")

if adapters is not None:

if alphas is None:

raise ValueError("`alphas` must be provided when `adapters` is specified")

assert len(alphas) == len(adapters), "`alphas` must be the same length as `adapters`"

for adapter, alpha in zip(adapters, alphas):

Copilot · 2025-09-15T09:31:30Z

tools/who_what_benchmark/whowhatbench/model_loaders.py

+            assert len(alphas) == len(adapter_names), "`alphas` must be the same length as `adapters`"
+            model.add_weighted_adapter(adapter_names, alphas, "merged_lora")
+        else:
+            model.add_weighted_adapter(adapter_names, "merged_lora")


The add_weighted_adapter method expects weights as the second parameter, but here a string 'merged_lora' is passed. This should likely be model.add_weighted_adapter(adapter_names, [1.0] * len(adapter_names), 'merged_lora') to provide equal weights when alphas are not specified.

Suggested change

model.add_weighted_adapter(adapter_names, "merged_lora")

model.add_weighted_adapter(adapter_names, [1.0] * len(adapter_names), "merged_lora")

likholat requested review from sbalandi and andreyanufr September 11, 2025 14:41

wwb text gen with lora

4f7db1f

github-actions bot added the category: WWB PR changes WWB label Sep 11, 2025

Wovchena requested a review from Copilot September 11, 2025 15:05

Copilot AI reviewed Sep 11, 2025

View reviewed changes

sbalandi reviewed Sep 12, 2025

View reviewed changes

review fixes

602ac01

sbalandi reviewed Sep 12, 2025

View reviewed changes

codestyle fix

68c20f8

Wovchena requested a review from Copilot September 15, 2025 09:30

Copilot AI reviewed Sep 15, 2025

View reviewed changes

andreyanufr approved these changes Sep 23, 2025

View reviewed changes

	if 'adapters' in kwargs and kwargs['adapters'] is not None:
	if kwargs.get("adapters") is not None:

	if "adapters" in kwargs and kwargs["adapters"] is not None:
	if kwargs.get("adapters") is not None:

	assert len(kwargs['alphas']) == len(kwargs["adapters"]), "`alphas` must be the same length as `adapters`"
	assert len(kwargs['alphas'], []) == len(kwargs["adapters"]), "`alphas` must be the same length as `adapters`"

-    if kwargs.get("adapters") is not None:
-        assert len(kwargs['alphas']) == len(kwargs["adapters"]), "`alphas` must be the same length as `adapters`"
-        for adapter, alpha in zip(kwargs['adapters'], kwargs['alphas']):
+    adapters = kwargs.get("adapters")
+    alphas = kwargs.get("alphas")
+    if adapters is not None:
+        if alphas is None:
+            raise ValueError("`alphas` must be provided when `adapters` is specified")
+        assert len(alphas) == len(adapters), "`alphas` must be the same length as `adapters`"
+        for adapter, alpha in zip(adapters, alphas):

	model.add_weighted_adapter(adapter_names, "merged_lora")
	model.add_weighted_adapter(adapter_names, [1.0] * len(adapter_names), "merged_lora")

WWB Text Generation with LoRA #2723

Are you sure you want to change the base?

WWB Text Generation with LoRA #2723

Conversation

likholat commented Sep 11, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Sep 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 11, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sbalandi Sep 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Sep 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 15, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sbalandi Sep 15, 2025 •

edited

Loading