Skip to content
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

[Tensor] Zero point support for unsigned int type  #2940

Merged
merged 1 commit into from
Feb 20, 2025

Conversation

djeong20
Copy link
Contributor

This pull request updates the unsigned int type Tensor class to include zero points in its memory management.
It's important to note that the size of the scale factors matches the size of the zero points.
At present, the zero point uses a scale factor size, which will be modified soon.

Self-evaluation:

  1. Build test: [X]Passed [ ]Failed [ ]Skipped
  2. Run test: [X]Passed [ ]Failed [ ]Skipped

Copy link
Contributor

@baek2sm baek2sm left a comment

Choose a reason for hiding this comment

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

Nice work. LGTM!

@djeong20 djeong20 added the PR/OK2GO CI failed but okay to merge/review label Feb 12, 2025
Copy link
Contributor

@EunjuYang EunjuYang left a comment

Choose a reason for hiding this comment

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

Overall looks good ! Please check the type of pointer:

Comment on lines 506 to 512
// copy scale factors
float *scales = (float *)(((int16_t *)buf) + size());
scopy(scale_size(), scales, 1, (float *)getScale(), 1);

// copy zero points
unsigned int *zps =
(unsigned int *)((float *)(((int16_t *)buf) + size()) + scale_size());
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// copy scale factors
float *scales = (float *)(((int16_t *)buf) + size());
scopy(scale_size(), scales, 1, (float *)getScale(), 1);
// copy zero points
unsigned int *zps =
(unsigned int *)((float *)(((int16_t *)buf) + size()) + scale_size());
// copy scale factors
float *scales = (float *)(((T *)buf) + size());
scopy(scale_size(), scales, 1, (float *)getScale(), 1);
// copy zero points
unsigned int *zps =
(unsigned int *)((float *)(((T *)buf) + size()) + scale_size());

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you! You are the best 👍

This pull request updates the unsigned int type Tensor class to include zero points in its memory management.
It's important to note that the size of the scale factors matches the size of the zero points.
At present, the zero point uses a scale factor size, which will be modified soon.

**Self-evaluation:**
1. Build test: [X]Passed [ ]Failed [ ]Skipped
2. Run test:   [X]Passed [ ]Failed [ ]Skipped

Signed-off-by: Donghyeon Jeong <[email protected]>
@djeong20 djeong20 force-pushed the tensor/uint/zero_point branch from 5783b38 to e4a1725 Compare February 13, 2025 02:35
Copy link
Member

@skykongkong8 skykongkong8 left a comment

Choose a reason for hiding this comment

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

Nice work!

Copy link
Contributor

@EunjuYang EunjuYang left a comment

Choose a reason for hiding this comment

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

LGTM ! 👍

Copy link
Collaborator

@jijoongmoon jijoongmoon left a comment

Choose a reason for hiding this comment

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

LGTM

@jijoongmoon jijoongmoon merged commit c1e5c60 into nnstreamer:main Feb 20, 2025
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR/OK2GO CI failed but okay to merge/review PR/READY2MERGE
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants