Qonnx binary quant dev #1355
Qonnx binary quant dev #1355
Conversation
…is not applicable for BinaryQuantizer since it does not have a scaling factor
…ls4ml into qonnx_binary_quant
… since it was already removed.
This reverts commit 6a74bfb.
…zations, since it was already removed." This reverts commit 89e2136.
removed irrelevant _AUTO_PO2 constant.
…the names aren't changed in between
…is parrallel existed before)
|
Nominally this is now ready and passes the tests when I ran manually, so I am setting ready for review. One thing I want to confirm, though, is that the non-unit scale tests actually test the proper handling of the scale, i.e. that the scales actually make a difference. |
|
Note, some of the fixes are general bnn fixes, not just BipolarQuant fixes. |
I did confirm at least that the outputs are different for the three different models when changing the scale, so although far from exhaustive, it does seem like the scale is at least handled for these models. |
| @@ -693,6 +693,11 @@ def replace_node(self, old_node, new_node): | |||
| repl = {old_name: new_name for old_name, new_name in zip(old_node.outputs, new_node.outputs)} | |||
| repl.update({old_name: new_name for old_name, new_name in zip(old_node.inputs, new_node.inputs)}) | |||
|
|
|||
| for old_output in old_node.outputs: | |||
There was a problem hiding this comment.
Is there a reason for moving this block around?
|
|
||
|
|
||
| class BipolarQuantConstantParameters(OptimizerPass): | ||
| """Remove Constant from the BipolarQaunt node parameters (but not input[0])""" |
| bias = np.array(0) # zeropt not defined for bipolar quants | ||
|
|
||
| # caclucate the new value -- actually not needed because bias == 0 | ||
| # new_val = const_node.get_attr('value') / scale + bias |
| @@ -0,0 +1,250 @@ | |||
| """ | |||
There was a problem hiding this comment.
The stuff in here are quite significant overlap with the normal quant_opt.py. I am wondering if it would be better to merge the two and handle the bipolar stuff as as special cases in the same optimizers. But that would result in a bunch of if statements, so I'm not sure which option would be cleaner and easier to maintain.
Description
This is an further work on PR #1292: Added a transformation for the QONNX operator BipolarQuant to hls4ml operator BinaryQuantizer.
Adding those changes showed issues in hls4ml binary handling, that this PR continues to fix.
Also included is treating
IntQuantas an alias forQuant.Type of change
For a new feature or function, please create an issue first to discuss it
with us before submitting a pull request.
Note: Please delete options that are not relevant.
Tests
This PR includes the BipolarQuant tests added to
test_qonnx.pyfrom the original PR (PR #1292).Checklist
pre-commiton the files I edited or added.