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_tall 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,ibeing width and int width of input precision, for the average pool. -
or, let all implementations use
accum_tto store intermediate values when reducing the pool. However, the user would need to set the precision manually in this case.