-
Notifications
You must be signed in to change notification settings - Fork 75
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] CharTensor class with signed 8-bit integer #2529
Conversation
📝 TAOS-CI Version: 1.5.20200925. Thank you for submitting PR #2529. Please a submit 1commit/1PR (one commit per one PR) policy to get comments quickly from reviewers. Your PR must pass all verificiation processes of cibot before starting a review process from reviewers. If you are new member to join this project, please read manuals in documentation folder and wiki page. In order to monitor a progress status of your PR in more detail, visit http://ci.nnstreamer.ai/. |
cibot: @djeong20, The last line of a text file must have a newline character. Please append a new line at the end of the line in nntrainer/tensor/char_tensor.cpp. |
cibot: @djeong20, A builder checker could not be completed because one of the checkers is not completed. In order to find out a reason, please go to http://ci.nnstreamer.ai/nntrainer/ci/repo-workers/pr-checker/2529-202404021623490.39387202262878-7bb62ef3aa6d943eb2683c5b06ae6fadae403931/. |
7bb62ef
to
50752a8
Compare
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.
@djeong20, 💯 All CI checkers are successfully verified. Thanks.
50752a8
to
dd3b891
Compare
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.
@djeong20, 💯 All CI checkers are successfully verified. Thanks.
dd3b891
to
cf08ee7
Compare
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.
@djeong20, 💯 All CI checkers are successfully verified. Thanks.
nntrainer/tensor/char_tensor.cpp
Outdated
case Initializer::ONES: | ||
setValue(1.0f); | ||
break; | ||
case Initializer::LECUN_NORMAL: |
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.
Could you check whether these initializers also work for int datatype?
If it is not, then it gives proper notification.
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.
currently, these initializers won't work for tensors with integer type and throw an error. I intentionally left the code for future work. however, if these initializers are not valid for integers, they can be removed such as follows.
case Initializer::LECUN_NORMAL: | |
case default: | |
throw std::invalid_argument("Initializer not valid in current data type"); |
I'll do some research and update this in the later PR!
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 figured both normal and uniform are not possible for integer types, so I made the changes. PTAL!
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.
overall LGTM
In this PR, a new type of tensor, the CharTensor class, is designed explicitly for handling signed 8-bit integer data types that have already undergone quantization. This new tensor class aims to provide users with more options when working with tensors and their respective data types. Currently, the CharTensor class does not support mathematical operations like multiplication or addition. However, these features will be added in future updates. **Self-evaluation:** 1. Build test: [X]Passed [ ]Failed [ ]Skipped 2. Run test: [X]Passed [ ]Failed [ ]Skipped Signed-off-by: Donghyeon Jeong <[email protected]>
cf08ee7
to
50cfa00
Compare
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.
@djeong20, 💯 All CI checkers are successfully verified. Thanks.
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!
In this PR, a new type of tensor, the CharTensor class, is designed explicitly for handling signed 8-bit integer data types that have already undergone quantization.
This new tensor class aims to provide users with more options when working with tensors and their respective data types.
Currently, the CharTensor class does not support mathematical operations like multiplication or addition. However, these features will be added in future updates.
Self-evaluation: