fix qonnx review suggestions

Author: jmitrevs

hls4ml/converters/__init__.py

@@ -11,8 +11,6 @@
 from hls4ml.converters.keras_to_hls import parse_keras_model  # noqa: F401
 from hls4ml.converters.keras_to_hls import keras_to_hls, register_keras_layer_handler
 from hls4ml.converters.onnx_to_hls import parse_onnx_model  # noqa: F401
-
-# from hls4ml.converters.pytorch_to_hls import parse_pytorch_model  # noqa: F401
 from hls4ml.model import ModelGraph
 from hls4ml.utils.config import create_config
 from hls4ml.utils.symbolic_utils import LUTFunction

hls4ml/converters/onnx/core.py

@@ -29,7 +29,6 @@ def parse_matmul_layer(node, input_names, input_shapes, graph):
     'Softmax',
     'Softsign',
     'Softplus',
-    # 'Clip',
 ]
 
 activation_map = {
@@ -45,7 +44,6 @@ def parse_matmul_layer(node, input_names, input_shapes, graph):
     'Softmax': 'Softmax',
     'Softsign': 'Activation',
     'Softplus': 'Activation',
-    # 'Clip': 'Clip',
 }
 # ---------
 
@@ -69,18 +67,6 @@ def parse_activation_layer(node, input_names, input_shapes, graph):
             layer['activation'] = layer['class_name']
             layer['activ_param'] = get_onnx_attribute(node, 'alpha', 0.01)
 
-        # # Don't yet support Clip
-        # elif layer['class_name'] == 'Clip':
-        #     clip_min_node = [x for x in graph.initializer if x.name in input_names]
-        #     clip_min = clip_min_node[0].float_data[0]
-
-        #     # Check if it's relu or not
-        #     if clip_min == 0.0:
-        #         layer['class_name'] = 'Activation'
-        #         layer['activation'] = 'ReLU'
-        #     else:
-        #         raise Exception('Clip with min != 0 is not supported yet!')
-
         else:
             layer['activation'] = layer['class_name']
             layer['class_name'] = 'Activation'

hls4ml/model/optimizer/passes/batchnorm_opt.py

@@ -30,7 +30,7 @@ def transform(self, model, node):
 
         gamma_node = node.get_input_node(node.inputs[1])
         if not isinstance(gamma_node, Constant):
-            raise TypeError('Only consant gammas supported')
+            raise TypeError('Only constant gammas supported')
         gamma = gamma_node.attributes['value']
         attributes['gamma_data'] = gamma
         attributes['gamma_quantizer'] = gamma_node.get_attr('quantizer')
@@ -40,7 +40,7 @@ def transform(self, model, node):
 
         beta_node = node.get_input_node(node.inputs[2])
         if not isinstance(beta_node, Constant):
-            raise TypeError('Only consant betas supported')
+            raise TypeError('Only constant betas supported')
         beta = beta_node.attributes['value']
         attributes['beta_data'] = beta
         attributes['beta_quantizer'] = beta_node.get_attr('quantizer')
@@ -49,7 +49,7 @@ def transform(self, model, node):
 
         moving_mean_node = node.get_input_node(node.inputs[3])
         if not isinstance(moving_mean_node, Constant):
-            raise TypeError('Only consant moving_means supported')
+            raise TypeError('Only constant moving_means supported')
         moving_mean = moving_mean_node.attributes['value']
         attributes['mean_data'] = moving_mean
         attributes['mean_quantizer'] = moving_mean_node.get_attr('quantizer')
@@ -58,7 +58,7 @@ def transform(self, model, node):
 
         moving_variance_node = node.get_input_node(node.inputs[4])
         if not isinstance(moving_variance_node, Constant):
-            raise TypeError('Only consant moving_variances supported')
+            raise TypeError('Only constant moving_variances supported')
         moving_variance = moving_variance_node.attributes['value']
         attributes['variance_data'] = moving_variance
         attributes['variance_quantizer'] = moving_variance_node.get_attr('quantizer')
@@ -147,12 +147,14 @@ def transform(self, model, node):
 
 class FuseConsecutiveBatchNormalization(OptimizerPass):
     """
-    OptimizerPass to merge consecutive BatchNormalization layers,
-    only if the earlier one does not have quantization specified
+    OptimizerPass to merge consecutive BatchNormalization layers, only if the earlier one does not have the output type
+    specified. There is a further check on the compatibility to merge: except in cases when merging a scale of 1 or a
+    bias of 0, this does not merge when both scales or both biases are quantized.
 
     Note:  Consider restricting this to ApplyAlpha.  Batch Normalization-style quantization seems to be ignored.
 
-    Note:  This optimizer may not be safe if weights are updateable. May need to turn off.
+    Note:  This optimizer may not be safe if weights are updateable, in particular if a scale can go from ones to other
+    values or if a bias can go from zeros to other values.
     """
 
     def match(self, node):
@@ -190,11 +192,6 @@ def transform(self, model, node):
         if len(prev_map[prev_node.outputs[0]]) > 1:
             return False
 
-        # # Not sure why this part is needed
-        # node_map = node.get_output_use_map()
-        # if len(node_map[node.outputs[0]]) > 1:
-        #     return False
-
         s0 = prev_node.weights['scale'].data_unquantized
         b0 = prev_node.weights['bias'].data_unquantized
         s1 = node.weights['scale'].data_unquantized

hls4ml/model/optimizer/passes/bn_fuse.py

@@ -7,8 +7,10 @@
 
 class FuseBatchNormalization(OptimizerPass):
     """
-    OptimizerPass to merge BatchNormalization layers,
-    only if the earlier one does not have quantization specified
+    OptimizerPass to merge a BatchNormalization layer with Dense or Conv layer, only if the Dense or Conv layer does not
+    have the output type specified. There is a further check on the compatibility to merge: except in cases when merging a
+    weight/scale of 1 or a bias of 0, this optimizer does not merge nodes when both the weight and scale or both biases
+    are quantized.
 
     Note:  Consider restricting this to ApplyAlpha.  Batch Normalization quantization seems to be ignored.
 
@@ -49,11 +51,6 @@ def transform(self, model, node):
         if len(parent_map[parent_node.outputs[0]]) > 1:
             return False
 
-        # # Not sure why this part is needed
-        # node_map = node.get_output_use_map()
-        # if len(node_map[node.outputs[0]]) > 1:
-        #     return False
-
         parent_weight = parent_node.weights['weight']
         parent_bias = parent_node.weights['bias']