Inconsistant behaviour of `accum_t` in `io_stream` and `io_parallel` for `pooling` layers with `Vivado` backend
Created by: calad0i
Prerequisites
Please make sure to check off these prerequisites before submitting a bug report.
-
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. -
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))>
, withb,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.