Skip to content

[core] Change u2 values packing in byte #31181

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

Jump to bottom
Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

[core] Change u2 values packing in byte #31181

wants to merge 5 commits into from
+195 −170

Conversation

praasz
Copy link
Contributor

@praasz praasz commented Jul 2, 2025 •
edited
Loading

Details:

  • Change u2 values packing in byte to be aligned with ONNX format.
  • Update Constant and Convert operators (tests)

Tickets:

@praasz praasz requested review from a team as code owners July 2, 2025 13:30
@github-actions github-actions bot added category: Core OpenVINO Core (aka ngraph) category: TEMPLATE OpenVINO Template plugin labels Jul 2, 2025
@praasz praasz changed the title [core ]Change u2 bit packing [core] Change u2 values packing in byte Jul 3, 2025
@praasz praasz added this to the 2025.3 milestone Jul 3, 2025
@praasz praasz requested review from alexsu52 and gkrivor July 3, 2025 06:02
alexsu52
alexsu52 approved these changes Jul 3, 2025
View reviewed changes
Copy link
Contributor

@alexsu52 alexsu52 left a comment

Choose a reason for hiding this comment

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

LGTM

t-jankowski
t-jankowski approved these changes Jul 3, 2025
View reviewed changes
@praasz praasz requested a review from PatrikStepan July 3, 2025 09:12
gkrivor
gkrivor approved these changes Jul 4, 2025
View reviewed changes
Copy link
Contributor

@gkrivor gkrivor left a comment

Choose a reason for hiding this comment

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

I've checked implementation vs onnxruntime as reference - with this change it infers as expected using CPU, GPU and TEMPLATE plugins.

@PatrikStepan
Copy link
Contributor

@ZoranZomborat can you please review this from the NPU side?

ZoranZomborat
ZoranZomborat approved these changes Jul 4, 2025
View reviewed changes
Copy link
Contributor

@ZoranZomborat ZoranZomborat left a comment

Choose a reason for hiding this comment

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

This is now also aligned with the NPU way of LSB->MSB packing! LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@t-jankowski t-jankowski t-jankowski approved these changes

@alexsu52 alexsu52 alexsu52 approved these changes

@ZoranZomborat ZoranZomborat ZoranZomborat approved these changes

@gkrivor gkrivor gkrivor approved these changes

@PatrikStepan PatrikStepan Awaiting requested review from PatrikStepan

Assignees

@alexsu52 alexsu52

@gkrivor gkrivor

Labels
category: Core OpenVINO Core (aka ngraph) category: TEMPLATE OpenVINO Template plugin do_not_merge
Projects
None yet
Milestone
2025.3
Development

Successfully merging this pull request may close these issues.
6 participants
@praasz @PatrikStepan @alexsu52 @ZoranZomborat @gkrivor @t-jankowski