hls4ml icon indicating copy to clipboard operation
hls4ml copied to clipboard

Inconsistant behaviour of `accum_t` in `io_stream` and `io_parallel` for `pooling` layers with `Vivado` backend

Open calad0i opened this issue 1 year ago • 0 comments

Prerequisites

Please make sure to check off these prerequisites before submitting a bug report.

  • [x] Test that the bug appears on the current version of the master branch. Make sure to include the commit hash of the commit you checked out.
  • [x] Check that the issue hasn't already been reported, by checking the currently open issues.
  • [ ] If there are steps to reproduce the problem, make sure to write them down below.
  • [ ] If relevant, please include the hls4ml project files, which were created directly before and/or after the bug.

Quick summary

For pooling operations with vivado (or vitis) backend and io_parallel, accum_t is defined but not used. With io_stream though, the intermediate values when reducing the pool are stored in an array of type accum_t.

Worth mentioning here that quartus not define accum_t at all, but uses value_type for intermediate values.

Expected behavior

Behaviour of accum_t should be consistent with different io types, (and backends, ideally).

Actual behavior

As stated.

Optional

Possible fix

  • purge accum_t all at once, as the necessary precision can be derived from input precision: directly input precision for max pool, and (u)fixed<b+2*ceil(log2(N)), i+1*ceil(log2(N))>, with b,i being width and int width of input precision, for the average pool.

  • or, let all implementations use accum_t to store intermediate values when reducing the pool. However, the user would need to set the precision manually in this case.

calad0i avatar Nov 05 '23 23:11 calad0i