hls4ml icon indicating copy to clipboard operation
hls4ml copied to clipboard

Make im2col default option for quartus

Open calad0i opened this issue 1 year ago • 3 comments

A# Description

With using the Quartus backend, with conv2d with kernel size 3x3 or conv1d with kernel size 3, the framework makes the default implementation Winograd instad of im2col. As the current Winograd implementation does not seem to play nicely with low bitwidth and hard to achieve bit-accuracy (e.g., with automatic precision setting), I would suggest not enable it by default.

For the Winograd impl, type uint8_t from <cstdint> is used. This header is not included by default in many environments, explicit #include <cstdint> is added to corresponding files.

Type of change

  • [x] Bug fix (non-breaking change that fixes an issue)
  • [ ] Documentation update
  • [ ] New feature (non-breaking change which adds functionality)
  • [x] Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • [ ] A new research paper code implementation
  • [ ] Other (Specify)

Tests

Checklist

  • [ ] I have read the guidelines for contributing.
  • [ ] I have commented my code, particularly in hard-to-understand areas.
  • [ ] I have made corresponding changes to the documentation.
  • [ ] My changes generate no new warnings.
  • [ ] I have installed and run pre-commit on the files I edited or added.
  • [ ] I have added tests that prove my fix is effective or that my feature works.

calad0i avatar May 03 '24 21:05 calad0i

I don't have a strong opinion on this. Let's see what others think.

jmitrevs avatar May 03 '24 23:05 jmitrevs

I'm fine with this type of change. But last I remember, im2col often didn't even synthesize so we added some heuristic to at least try winograd if possible. @bo3z might remember better.

vloncar avatar May 04 '24 08:05 vloncar

I'm fine with this type of change. But last I remember, im2col often didn't even synthesize so we added some heuristic to at least try winograd if possible. @bo3z might remember better.

Generally, some designs would fail HLS compilation and it just seemed that Winograd was more successful. This could be tied to a specific version of Intel HLS or some implementation detail we had at the time. If I recall correctly, the heuristic was such that if it's 3x3 (and no stride, padding etc.) we set it to use Winograd. But it's well known that Winograd doesn't perform well at extreme quantisations (which granted were never tested at the time).

bo3z avatar May 05 '24 13:05 bo3z