Leaky Quantized ReLU fix #961

DrWatt · 2024-01-29T12:00:03Z

Description

The quantized_relu activation layer included in the QKeras library lets the user set the negative_slope option which practically change the activation function from the usual ReLU to a LeakyReLU. This change was not perceived by hls4ml, meaning that that information was lost when the HLS_model was created by the library.
In order to fix this behaviour I have added the QLeakyReLU in the list of supported layers in model/layers.py using the ParametrizedActivation implementation mimicking the LeakyReLU already present and so following a similar path of implementation to the "non-leaky" quantized_relu.
Other changes were made to model/profiling.py and utils/config.py to make them compatible with the new layer.

A couple of other fixes were made in backends/vivado_accelerator/vivado_accelerator_config.py (a missing "casting" function when comparing complex objects with strings) and in model/types.py (adding the ap_ to the hls fixed precision type due to errors raised by the vivado compiler)

Type of change

Bug fix (non-breaking change that fixes an issue)

Tests

The quantized_relu with a negative_slope different from the default one was added in the pytest routine covering qkeras' layers.
A specific test script has been also added to the pytest directory. The results from the new implementation are asserted to be equal to the QKeras layer with a relative tolerance of 0.00001.

Checklist

I have read the guidelines for contributing.
I have commented my code, particularly in hard-to-understand areas.
I have made corresponding changes to the documentation.
My changes generate no new warnings.
I have installed and run pre-commit on the files I edited or added.
I have added tests that prove my fix is effective or that my feature works.

… new layer easier

… and default values to general attributes

…near activation embedded in qkeras layers without activation in k_to_hls converter

…ot produce leakyrelu activations when negative slope is provided

… the layer templates

…thers test scripts

jmitrevs · 2024-02-07T18:21:57Z

hls4ml/model/types.py

@@ -300,7 +300,7 @@ def saturation_mode(self, mode):
    def __str__(self):
        args = [self.width, self.integer, self.rounding_mode, self.saturation_mode, self.saturation_bits]
        args = ','.join([str(arg) for arg in args if arg is not None])
-        typestring = '{signed}fixed<{args}>'.format(signed='u' if not self.signed else '', args=args)
+        typestring = 'ap_{signed}fixed<{args}>'.format(signed='u' if not self.signed else '', args=args)


We don't have 'ap_' or 'ac_' prefixes in our internal typestings. This change should be reverted

I have removed the ap_ here and fixed the vivado_accelerator writer which gave issues with the compiler due to the my_project_axi.hpp not using the hls type definition but directly the internal typestrings. However this fix has been removed from this branch to separate the fixes.

hls4ml/utils/config.py

…r regarding ap_fixed types temporarily directly in the vivado accelerator writer

… not according to the negative slope

vloncar

Looks good, needs cleanup. Thanks Marco!

vloncar · 2024-02-08T13:49:27Z

.gitignore

@@ -14,3 +14,4 @@ docs/autodoc/*
 hls4mlprj_*
 *~
 *.ipynb_checkpoints/
+*.bak


Unlrelated change

Exception removed, my bad, it was here for the byproducts of running precommit manually.

vloncar · 2024-02-08T13:49:48Z

hls4ml/backends/vivado_accelerator/vivado_accelerator_config.py

@@ -68,16 +68,16 @@ def __init__(self, config, model_inputs, model_outputs):
        if out_axi_t not in ['float', 'double']:
            self.output_type = self._next_factor8_type(config.backend.convert_precision_string(out_axi_t))

-        if self.input_type == 'float':
+        if str(self.input_type) == 'float':


Unlrelated change, shouldn't be a part of this PR

I have removed all the fixes concerning the vivado_accelerator backend and writer (small stuff), should I open another PR for those?

vloncar · 2024-02-08T13:50:33Z

hls4ml/converters/keras/qkeras.py

@@ -147,6 +146,10 @@ def parse_qactivation_layer(keras_layer, input_names, input_shapes, data_reader)
        layer['slope_prec'] = FixedPrecisionType(width=2, integer=0, signed=False)
        layer['shift_prec'] = FixedPrecisionType(width=2, integer=0, signed=False)
        layer['activation'] = activation_config['class_name'].replace('quantized_', 'hard_')
+    elif activation_config['class_name'] == 'quantized_relu' and activation_config['config']['negative_slope'] != 0:
+        layer['class_name'] = 'QLeakyReLU'


Calling it QLeakyReLU is only complicating things down the line. Without Q there's no need for changes to keras_to_hls.py and layers.py

You are right, I wanted to be clear when writing the code for my sake but indeed using LeakyReLU makes the fix simpler.

vloncar · 2024-02-08T13:50:50Z

hls4ml/converters/keras_to_hls.py

@@ -208,6 +208,8 @@ def parse_keras_model(model_arch, reader):
    ]
    # Recurrent layers
    recurrent_layers = ['SimpleRNN', 'LSTM', 'GRU']
+    # QActivation layers


Not needed with the change above

vloncar · 2024-02-08T13:51:10Z

hls4ml/converters/keras_to_hls.py

-        if 'activation' in layer and layer['class_name'] not in activation_layers + recurrent_layers:  # + qkeras_layers:
+        if (
+            'activation' in layer
+            and layer['class_name'] not in activation_layers + recurrent_layers + qactivation_layers


vloncar · 2024-02-08T14:00:02Z

hls4ml/utils/config.py

-            signed = False
-            integer -= 1
+            if quantizer['config']['negative_slope'] != 0.0:
+                print(quantizer)


get rid of the print statement and commented out code

Print and commented code removed

vloncar · 2024-02-08T14:00:18Z

hls4ml/writer/vivado_accelerator_writer.py

@@ -38,8 +38,14 @@ def write_axi_wrapper(self, model):
                newline += f'static const unsigned N_IN = {inp.size()};\n'
                newline += f'static const unsigned N_OUT = {out.size()};\n'
                if self.vivado_accelerator_config.get_interface() == 'axi_stream':
-                    newline += f'typedef {inp_axi_t} T_in;\n'
-                    newline += f'typedef {out_axi_t} T_out;\n'
+                    if 'fixed' in str(inp_axi_t):


unlrelated to this PR

vloncar · 2024-02-08T14:01:28Z

test/pytest/q_leaky_relu_test.py

+def test_quantizer(randX_1000_1, quantizer, backend, io_type):
+    '''
+    Test a single quantizer as an Activation function.
+    Checks the type inference through the conversion is correct without just


Not sure I understand this sentence :-)

I am sorry, it does not make any sense, I think it was a copy and paste mishap.

vloncar · 2024-02-08T14:02:46Z

test/pytest/q_leaky_relu_test.py

+)
+@pytest.mark.parametrize('backend', ['Vivado', 'Vitis', 'Quartus'])
+@pytest.mark.parametrize('io_type', ['io_parallel', 'io_stream'])
+def test_quantizer(randX_1000_1, quantizer, backend, io_type):


This should have a proper name, and could be fused with the existing qkeras test file. Or split along with the other qactivation tests. But whatever the solution ends up being it should be properly named with test_ prefix.

vloncar · 2024-02-08T14:03:30Z

test/pytest/test_qkeras.py

@@ -272,6 +272,7 @@ def randX_1000_1():
        (quantized_relu(8, 4)),
        (quantized_relu(10)),
        (quantized_relu(10, 5)),
+        (quantized_relu(10, 5, negative_slope=0.25)),


Aren't you already testing this in the other test?

In order to create the test suite requested for the PR I added the line in the already present qkeras test, however due to problems with the environment requested for the pytests, some issues not concerning my fix came up. So, I have written a separate script just to show that the "new" layer passes an accuracy test. If the test_qkeras.py test passes when called in the automatic workflow, my other script can be removed.

jmitrevs · 2024-03-08T23:55:30Z

Sorry for the delay, but what is the status of this? It looks to me like maybe I can approve and merge it.

vloncar · 2024-03-28T08:28:03Z

I was testing this and the changes work, however the PR includes unnecessary changes and the test doesn't do anything (data is all positive, so it passes even without the changes in the converter). I distilled the changes and made a proper test in https://github.com/vloncar/hls4ml/tree/qrelu_negative_slope

vloncar · 2024-03-28T20:55:17Z

I made #987 as a continuation of this PR

DrWatt and others added 24 commits March 29, 2023 18:36

First attempt in making a layer generator for hls4ml to make adding a…

4e483fc

… new layer easier

Added longnamelayer to associate hls layer to TF layer and added type…

0e973e6

… and default values to general attributes

Merge branch 'fastmachinelearning:main' into master

ebbac33

Merge branch 'fastmachinelearning:main' into master

e1e9157

Fixed a missing casting in vivado_accelerator and added a skip for li…

49a30fe

…near activation embedded in qkeras layers without activation in k_to_hls converter

Fixed missing _ap_ in typestring sometimes not supported by the compiler

e73ddc9

Fixed issue with quantized_relu activation whose implementation did n…

6db982a

…ot produce leakyrelu activations when negative slope is provided

Removed a couple of prints used in debugging

6ea8ab0

Merge branch 'fastmachinelearning:main' into master

a13c978

testing test

6640169

pre-commit ran manually on all files

6028bb6

Fixed issues arised due to an old version of hls4ml used when writing…

6c72dd9

… the layer templates

Fixed hls header name

61ae795

first hls compilation errors solved in new layer template

7b4047d

Readme added for the New Layer tool

235aa5a

Added notebook for testing quickly a new layer

99afa33

Changed name of directory

7dd760d

Old directory deleted

11371c5

Missing outputshape description in Readme

eed800d

Removing stuff that do not concern the bug fix

fde9841

Uncommented line in profiling

372d35d

Test suite added for quantized leaky relu

928f383

Changed output_dir of tests to follow the same configuration as the o…

3b561b4

…thers test scripts

Merge branch 'fastmachinelearning:main' into qrelu_fix

da65388

jmitrevs reviewed Feb 7, 2024

View reviewed changes

hls4ml/utils/config.py Outdated Show resolved Hide resolved

DrWatt added 2 commits February 8, 2024 12:00

Removed 'ap_' string from FixedPrecisionType and fixed compiling erro…

f1d187d

…r regarding ap_fixed types temporarily directly in the vivado accelerator writer

Added if clause in the quantized_relu config which makes it signed or…

700163b

… not according to the negative slope

vloncar requested changes Feb 8, 2024

View reviewed changes

Renamed QLeakyReLU to LeakyReLU and subsequent changes

3f48509

DrWatt and others added 6 commits February 9, 2024 11:34

Reversed changes regarding vivado_accelerator backend and writer

d209f5b

Merge branch 'fastmachinelearning:main' into qrelu_fix

46a6e9b

changed description string of qrelu test

26f32a1

Name of qleaky relu test changed

d4642e7

Name of qleaky relu test changed

b1a4472

Merge branch 'fastmachinelearning:main' into qrelu_fix

ecec240

vloncar mentioned this pull request Feb 22, 2024

Problem tracing binary CNN model after recent tracing optimization #974

Closed

4 tasks

jmitrevs added the please test Trigger testing by creating local PR branch label Feb 26, 2024

jmitrevs approved these changes Mar 18, 2024

View reviewed changes

Merge branch 'main' into qrelu_fix

34c8070

DrWatt requested a review from vloncar March 27, 2024 18:03

vloncar mentioned this pull request Mar 28, 2024

Support negative_slope in quantized_relu #987

Merged

vloncar closed this Mar 28, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Leaky Quantized ReLU fix #961

Leaky Quantized ReLU fix #961

DrWatt commented Jan 29, 2024

jmitrevs Feb 7, 2024

DrWatt Feb 9, 2024

vloncar left a comment

vloncar Feb 8, 2024

DrWatt Feb 9, 2024

vloncar Feb 8, 2024

DrWatt Feb 9, 2024

vloncar Feb 8, 2024

DrWatt Feb 9, 2024

vloncar Feb 8, 2024

vloncar Feb 8, 2024

vloncar Feb 8, 2024

DrWatt Feb 9, 2024

vloncar Feb 8, 2024

vloncar Feb 8, 2024

DrWatt Feb 9, 2024

vloncar Feb 8, 2024

vloncar Feb 8, 2024

DrWatt Feb 9, 2024

jmitrevs commented Mar 8, 2024

vloncar commented Mar 28, 2024

vloncar commented Mar 28, 2024

Leaky Quantized ReLU fix #961

Leaky Quantized ReLU fix #961

Conversation

DrWatt commented Jan 29, 2024

Description

Type of change

Tests

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vloncar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmitrevs commented Mar 8, 2024

vloncar commented Mar 28, 2024

vloncar commented Mar 28, 2024