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

NN_ROUND() issues in 16bit ops #1047

Closed
majianjia opened this issue Nov 20, 2020 · 4 comments
Closed

NN_ROUND() issues in 16bit ops #1047

majianjia opened this issue Nov 20, 2020 · 4 comments
Assignees

Comments

@majianjia
Copy link
Contributor

majianjia commented Nov 20, 2020

I met an issue of 16 bit NN producing bad results when I tried to use round() in NN calculation using ARM_NN_TRUNCATE (undefined ARM_NN_TRUNCATE)
I normally define ARM_NN_TRUNCATE but recently I want to try to use round so I undefined the macro. In 8 bit ops, it brings some accuracy improvement but in 16 bit it fails.

The 16bit ops I use is this one but other which uses NN_ROUND() have similar issues.

arm_fully_connected_mat_q7_vec_q15_opt(const q15_t * pV,

change #define NN_ROUND(out_shift) ( (0x1u << out_shift) >> 1 ) to
#define NN_ROUND(out_shift) ((0x1 << out_shift) >> 1 ) fixed my issue.

#define NN_ROUND(out_shift) ( (0x1u << out_shift) >> 1 )

Also, it doesn't seem right when it adds to a negative number. Should the round be round down in negative number? In most ops, only add this to the bias which means it is always positive.

More information will be provided when I figure it out. Thanks.

majianjia added a commit to majianjia/nnom that referenced this issue Nov 20, 2020
- The local backend seems has some issue with NNOM_ROUND()
ARM-software/CMSIS_5#1047
@felix-johnny
Copy link
Contributor

@majianjia Apologies for the delay. Would you mind making a PR for the fix?

@majianjia
Copy link
Contributor Author

majianjia commented Aug 13, 2021

@felix-johnny let me remind myself what is this about.
Happy to, thanks :p

@felix-johnny
Copy link
Contributor

Great , thanks!

majianjia added a commit to majianjia/CMSIS_5 that referenced this issue Aug 25, 2021
felix-johnny pushed a commit that referenced this issue Nov 2, 2021
@majianjia
Copy link
Contributor Author

majianjia commented Nov 2, 2021

close the issue as the related PR is merged.
Thanks

felix-johnny pushed a commit to ARM-software/CMSIS-NN that referenced this issue Oct 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants