-
Notifications
You must be signed in to change notification settings - Fork 244
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
[Release] Set environment variable on LNL CPUs before compression model compilation #3233
[Release] Set environment variable on LNL CPUs before compression model compilation #3233
Conversation
@@ -115,6 +117,17 @@ def clear_ov_model_cache(): | |||
OV_MODEL_CACHE.clear() | |||
|
|||
|
|||
def _compile_ov_model(model: ov.Model, device_name: str, config: Dict[str, str]) -> ov.CompiledModel: | |||
if is_lnl_cpu(): |
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.
Why do we need to set it for LNL CPU particularly and not in all cases? Is there any drawback?
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.
I was recommended not to set it always. @dmitry-gorokhov could you comment on the possible drawbacks?
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.
It has to be done on LNL only. Exporting this varibable on systems w/o AVX-VNNI support will result in illegal instruction errors. Besides that it will disable AMX/AVX512 isa on Xeons.
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.
I think we need to run more checks to make sure it's not breaking anything. At least SDL due to new dep and WC conformance tests.
def is_lnl_cpu() -> bool: | ||
global _IS_LNL_CPU | ||
if _IS_LNL_CPU is None: | ||
_IS_LNL_CPU = re.search(r"Ultra \d 2\d{2}", cpuinfo.get_cpu_info()["brand_raw"]) is not None |
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.
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.
LGTM, please address comments from the last review.
Co-authored-by: Alexander Suslov <[email protected]>
f94cd1e
into
openvinotoolkit:release_v2150
…el compilation (openvinotoolkit#3233) ### Changes Set `export DNNL_MAX_CPU_ISA=AVX2_VNNI` during weights compression on LNL CPUs. ### Reason for changes Fix compression on LNL CPUs. ### Related tickets 161336 ### Tests - https://github.com/openvinotoolkit/nncf/actions/runs/13075940656 ✅ - https://github.com/openvinotoolkit/nncf/actions/runs/13075938396 ✅ - NNCF/job/nightly/job/sdl/job/nightly_trigger/426/ ✅ --------- Co-authored-by: Alexander Suslov <[email protected]>
### Changes Follow-up to #3233 . Fix an issue arising during documentation building. ### Reason for changes Caught here: https://github.com/openvinotoolkit/nncf/actions/runs/13131916886/job/36638718655 . This test is run on PRs to develop only. That's why it was not caught during merging to release branch. ### Tests https://github.com/openvinotoolkit/nncf/actions/runs/13132576726/job/36640708447?pr=3246
…el compilation (openvinotoolkit#3233) ### Changes Set `export DNNL_MAX_CPU_ISA=AVX2_VNNI` during weights compression on LNL CPUs. ### Reason for changes Fix compression on LNL CPUs. ### Related tickets 161336 ### Tests - https://github.com/openvinotoolkit/nncf/actions/runs/13075940656 ✅ - https://github.com/openvinotoolkit/nncf/actions/runs/13075938396 ✅ - NNCF/job/nightly/job/sdl/job/nightly_trigger/426/ ✅ --------- Co-authored-by: Alexander Suslov <[email protected]>
### Changes Follow-up to openvinotoolkit#3233 . Fix an issue arising during documentation building. ### Reason for changes Caught here: https://github.com/openvinotoolkit/nncf/actions/runs/13131916886/job/36638718655 . This test is run on PRs to develop only. That's why it was not caught during merging to release branch. ### Tests https://github.com/openvinotoolkit/nncf/actions/runs/13132576726/job/36640708447?pr=3246
…tion (#3246) Copy of #3233 and #3247 to develop branch --------- Co-authored-by: Alexander Suslov <[email protected]>
Changes
Set
export DNNL_MAX_CPU_ISA=AVX2_VNNI
during weights compression on LNL CPUs.Reason for changes
Fix compression on LNL CPUs.
Related tickets
161336
Tests