search

harvard-acc / gem5-aladdin

End-to-end SoC simulation: integrating the gem5 system simulator with the Aladdin accelerator simulator.
BSD 3-Clause "New" or "Revised" License
210 stars 59 forks source link

systolic-array: Fix a tensor iterator bug. #37

Closed yaoyuannnn closed 3 years ago

yaoyuannnn commented 3 years ago

systolic-array: Fix a tensor iterator bug.

This also adds unit tests for the tensor index iterators.

The test/ folder that contains an example program is renamed to example/.

yaoyuannnn commented 3 years ago

@xyzsam This fixes a bug in the tensor iterator. Tested resnet50 and it worked. This piece of code that deals with offset/carry is complicated and ideally I would want to write some catch2 test cases, but let's check this in for now..

yaoyuannnn commented 3 years ago

@xyzsam This fixes a bug in the tensor iterator. Tested resnet50 and it worked. This piece of code that deals with offset/carry is complicated and ideally I would want to write some catch2 test cases, but let's check this in for now..

I'm pretty sure that if we don't add a test now, we're not going to in the future :) How much work is it to add a test for this? Any test that fails before this PR and passes after is better than nothing.

I guess you're right :) Let me add a few unit tests for this over this long weekend. Other than Catch2 boilerplate code, I just need a reference convolution implementation to validate the results.

yaoyuannnn commented 3 years ago

@xyzsam sorry, it breaks the build somehow, will let you know when it's ready for review.

yaoyuannnn commented 3 years ago

It's ready for review now.