-
Notifications
You must be signed in to change notification settings - Fork 409
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix GlobalPooling1D Layers #399
Conversation
One issue discovered while creating the tests is that io_stream GlobalAveragePooling has a tendency to overflow when doing the sum. Unlike the io_parallel implementation, it does not use a wider accumulator size. I am trying to see what is the best way to fix this. |
I added a test (and also an include of |
c5a19b9
to
1b42183
Compare
@thesps if you could review this, that'd be great. I think it's ready, except for perhaps creating an optimizer that would set
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, looks good to me now! On the point about setting the accumulator type - the automated setting of the accumulator type would be a good follow up, but I think what we have here is already good enough to merge since the type can be configured properly in any case.
I just changed the directory name that the test writes to from hls4ml_...
to hls4mlprj...
like the other tests. It's partly convention, but also writing rm -r hls4ml*
to clean up after running locally makes me nervous 😅
* add global_pooling1d_cl * io_parallel global_pooling1d_cl * add globalpooling1d testing; add missing include in nnet_conv_stream.h * update test * Update test_globalpooling1d.py * Change project directory name Co-authored-by: Jovan Mitrevski <[email protected]> Co-authored-by: Sioni Summers <[email protected]>
* add global_pooling1d_cl * io_parallel global_pooling1d_cl * add globalpooling1d testing; add missing include in nnet_conv_stream.h * update test * Update test_globalpooling1d.py * Change project directory name Co-authored-by: Jovan Mitrevski <[email protected]> Co-authored-by: Sioni Summers <[email protected]>
Address #398
Test script (to be turned into a real test): https://gist.github.com/jmduarte/99aee1eee92524ad3b1dbf5c5a863296
Changes to do:
global_pooling1d_cl
io_stream
HLS implementationglobal_pooling1d_cl
io_parallel
HLS implementation@jmitrevs