Skip to content
This repository has been archived by the owner on Dec 1, 2021. It is now read-only.

Add bug-fixed version of quantized convolution IP with kn2row and tiling #130

Merged
merged 46 commits into from
Feb 15, 2019
Merged

Add bug-fixed version of quantized convolution IP with kn2row and tiling #130

merged 46 commits into from
Feb 15, 2019

Conversation

ymd8bit
Copy link
Contributor

@ymd8bit ymd8bit commented Jan 25, 2019

Overview

This request mainly focuses on adding new quantized convolution IP called qconv_kn2row_tiling.
During my development, I've used linter in my environment (clang-format), so this seems to have many modified files that are not inherently important.
Also this request contains much context and some analysis I need to explain, so I described them in the following doc. Please quickly scan it, then check the modification if you'd like.
https://docs.google.com/document/d/1_WmWuJ2cRZ5ZOC3TBWMi220ji5kfN67jerBhDBXzTKg/edit?usp=sharing

@blueoilbot
Copy link

Can one of the admins verify this patch?

@ymd8bit ymd8bit changed the title [WIP] Add bug-fixed version of quantized convolution IP with kn2row and tiling Add bug-fixed version of quantized convolution IP with kn2row and tiling Jan 31, 2019
@n-nez
Copy link
Contributor

n-nez commented Jan 31, 2019

@tkclimb nice! 👍
will look more thoughtfully through it tomorrow...

@iizukak iizukak added the bug Something isn't working label Feb 9, 2019
@iizukak iizukak added this to the v0.4.0 milestone Feb 9, 2019
@antonionevado
Copy link
Contributor

@n-nez and me tried your branch with the correct .rbf and preloader manually. It works fine.
(we tried classification/lmnet_quantize_cifar10_space_to_depth with hq, ts and cache flags)

Seems that the testing code still using the old .rbf and preloader.
This path should also changed to point to the right hardware design:
https://github.com/tkclimb/blueoil/blob/add_qconv_kn2row_tiling/dlk/tests/testcase_dlk_base.py#L52

Maybe hw_path should point to dlk/hw/intel/de10_nano/qconv_kn2row_tiling/soc_system.rbf ? Please check that point.

@n-nez
Copy link
Contributor

n-nez commented Feb 15, 2019

run dlk test

@ruimashita
Copy link
Contributor

@n-nez @antonionevado @lm-lily
Please review 👍👍👍

@n-nez
Copy link
Contributor

n-nez commented Feb 15, 2019

@ruimashita Already approved it.

There are lots of unused code / unrelated changes under dlk/bakends, but we are going to fix it in following PRs. Instead of delaying this one. 😇

For the meaningful part of the code, everything is ok. 🙂

@n-nez n-nez merged commit e3d8252 into blue-oil:master Feb 15, 2019
@ruimashita
Copy link
Contributor

@n-nez
Because there are changes after your approved, I asked re-review again.

we are going to fix it in following PRs. Instead of delaying this one
For the meaningful part of the code, everything is ok

But it is nice, no problem, Thanks 👍👍👍

@n-nez
Copy link
Contributor

n-nez commented Feb 15, 2019

@ruimashita Yep, I know, I saw them too. 🙂
As it was @antonionevado @lm-konda and me who proposed these changes 😅 💪

ruimashita pushed a commit that referenced this pull request Jun 26, 2019
Add bug-fixed version of quantized convolution IP with kn2row and tiling
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants