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

Wrong detection results for yolov7 on RISC-V with RVV #5360

Closed
vassilijnadarajah opened this issue Mar 3, 2024 · 7 comments
Closed

Wrong detection results for yolov7 on RISC-V with RVV #5360

vassilijnadarajah opened this issue Mar 3, 2024 · 7 comments
Labels

Comments

@vassilijnadarajah
Copy link

error log | 日志或报错信息 | ログ

When peforming object detection with yolov7 and NCNN on RISC-V, I get different results depending on if i use NCNN with RVV support or without RVV support. I use the same source code but the NCNN library is compiled differently. The results are the same, when perfoming inference on x86, armv8 or RISC-V without RVV. Just when compiling with RVV, I get wrong results.

Classification with NCNN on RISC-V with RVV

failed_rvv_classification

Classification with NCNN on RISC-V without RVV

successful_classification

context | 编译/运行环境 | バックグラウンド

how to reproduce | 复现步骤 | 再現方法

I uploaded an example project with instructions on how to compile with and without RVV support. For the compilation of the NCNN library i followed the steps in the wiki.

more | 其他 | その他

Please let me know, if I need to supply more information. I would really appreciate any help with this. Thank you in advance! 谢谢!

@wzyforgit
Copy link
Contributor

Please run ncnn unit test with RVV to see what goes wrong.

Run ncnn unit test:
cmake -DNCNN_BUILD_TESTS=ON (other parameters) ..
make
make test

@vassilijnadarajah
Copy link
Author

Hi @wzyforgit! Thank you for taking the time to reply. I just ran all the unit tests and apart from one test, all of them have passed.

When running test_reduction, I get the error message param is too old, please regenerate! multiple times. I attached a screenshot.

Also for test_squeezenet I get overwrite built-in layer type Convolution and overwrite built-in layer type 6 printed to the console. The test still passes though.

I'd be thankful for any other tips you may be able to give me.

param_too_old

@wzyforgit
Copy link
Contributor

Hi @wzyforgit! Thank you for taking the time to reply. I just ran all the unit tests and apart from one test, all of them have passed.

When running test_reduction, I get the error message param is too old, please regenerate! multiple times. I attached a screenshot.

Also for test_squeezenet I get overwrite built-in layer type Convolution and overwrite built-in layer type 6 printed to the console. The test still passes though.

I'd be thankful for any other tips you may be able to give me.

param_too_old

There is another way here:
0. Keep use RVV param in cmake

  1. Remove all RVV files in src/layer/riscv
  2. Recover one RVV file in src/layer/riscv
  3. Build ncnn and run your yolov7 application
    4.1 If the detection results are wrong, the file you recovered last time have bug, remove it
    4.2 If the detection results are all right, the file you recovered last time is right, keep it
  4. Repeat steps 2 to 4 until you find all the problematic files
  5. Remove all problematic files and rebuild ncnn into your application

I'm sorry that I'm not good at RVV and C906 core.
If your project is urgent, you can try this method to get maximum speed up and wait for the owner to fix the problems.

@nihui
Copy link
Member

nihui commented Apr 1, 2024

param is too old, please regenerate! warning fixed in #5397
I am now trying to reproduce the rvv issue

@nihui nihui added the bug label Apr 1, 2024
@nihui
Copy link
Member

nihui commented Apr 1, 2024

The bug has been confirmed and can be reproduced

nihui added a commit to nihui/ncnn that referenced this issue Apr 1, 2024
@nihui
Copy link
Member

nihui commented Apr 1, 2024

fixed in #5398
Thanks for your report

@nihui nihui closed this as completed in 824b79a Apr 1, 2024
@vassilijnadarajah
Copy link
Author

Thank you so much @nihui! It works now :)

senli123 pushed a commit to senli123/ncnn that referenced this issue May 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants