Bidirectional RNN layer support for Keras frontend and Vitis backend #1310

enlupi · 2025-06-12T14:12:23Z

Description

This PR adds support for Bidirectional RNN layers using Keras with the Vitis backend in io_parallel mode. The forward and backward layer can be either LSTM or GRU, and their architecture and independent one from the other.

Type of change

New feature

Tests

Unit test in test/pytest/test_rnn.py was updated to also check parsing and accuracy for a Bidirectional layer.

Test Configuration:

The new tests are carried out using only Vivado or Vitis backend and io_parallel mode.

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.

…ctures

JanFSchulte · 2025-06-26T19:20:50Z

hls4ml/backends/vitis/passes/feature_check.py

+        print(
+            f'WARNING: The selected order for forward and backward layers in "{node.name}" ({node.class_name}) is not '
+            'supported in Vitis backend. Switching to forward layer first, backward layer last.'
+        )


Where does this switching actually happen? Or is this meant to prompt the user to do it themselves? Also, this probably should just be caught directly in the parser where the swapped_order attribute is determined.

JanFSchulte · 2025-06-26T19:24:39Z

hls4ml/backends/vitis/passes/feature_check.py

+            f'WARNING: "{merge_mode}" merge mode in "{node.name}" ({node.class_name}) is not supported in Vitis backend. '
+            'Switching to "concat" merge mode.'
+        )
+        node.set_attr('merge_mode', 'concat')


Why are we doing this here instead of just doing it during the parsing in the converter?

JanFSchulte · 2025-06-26T19:33:42Z

hls4ml/backends/vivado/passes/recurrent_templates.py

+        if params['pass_initial_states'] == 'true':
+            params['input2_t'] = node.get_input_variable(node.inputs[1]).type.name
+            params['input2'] = node.get_input_variable(node.inputs[1]).name
+            if node.class_name == 'BLSTM':


Should this be just LSTM? I don't see BLSTM as a class name anywhere else.

JanFSchulte · 2025-06-26T19:36:33Z

hls4ml/converters/keras/recurrent.py

+            temp_layer = rnn_forward_layer.copy()
+            rnn_forward_layer = rnn_backward_layer.copy()
+            rnn_backward_layer = temp_layer
+            swapped_order = True


I don't think this case is supported, right? We should probably just throw an exception here and tell the user.

JanFSchulte · 2025-06-26T19:40:54Z

hls4ml/converters/keras/recurrent.py

@@ -11,13 +11,15 @@
 )

 rnn_layers = ['SimpleRNN', 'LSTM', 'GRU']
+merge_modes = ['sum', 'mul', 'concat', 'ave']


Why list the other 3 here when only concat is supported?

JanFSchulte · 2025-06-26T19:44:26Z

hls4ml/templates/vivado/nnet_utils/nnet_recurrent.h

+        h_state = h_state_forward;
+        s_state = s_state_forward;
+    }
+*/


Please remove commented code.

JanFSchulte · 2025-06-26T19:44:48Z

hls4ml/templates/vivado/nnet_utils/nnet_recurrent.h

+    std::cout << "~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~" << std::endl << std::endl;
+    std::cout << "Data_t size: " << data_T::size << std::endl;
+    std::cout << std::endl << "~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~" << std::endl << std::endl;
+


Please remove these couts.

JanFSchulte · 2025-06-26T19:45:06Z

hls4ml/templates/vivado/nnet_utils/nnet_recurrent.h

+    else {
+        h_state = h_state_forward;
+    }
+*/


Please remove commented code.

JanFSchulte · 2025-06-26T19:52:20Z

Generally this looks good to me, comments are minor. I'll wait until some things are merged that should fix some tests failures and then run the CI.

rimalroc · 2025-06-27T10:01:06Z

Hi, thank you for implementing this, have you tried this with kerasv3? the mentioned test unit is using keras2 only
It seems to fall to the keras v2 handler, but I get the following error.

v2 handler used for layer bidirectional
Traceback (most recent call last):
  File "/work/NGT/ngt2.2-toy-simulation/./convert/test_convert.py", line 180, in <module>
    hls_model = converttools.conv_to_hls(models[mod_id], model,REWRITE_CONF=args.rewriteconf, verbose=True)
  File "/work/NGT/ngt2.2-toy-simulation/convert/../convert/converttools.py", line 211, in conv_to_hls
    hls_model = hls4ml.converters.convert_from_keras_model(
  File "/work/NGT/hls4ml_enlupi/hls4ml/hls4ml/utils/dependency.py", line 46, in inner
    return f(*args, **kwargs)
  File "/work/NGT/hls4ml_enlupi/hls4ml/hls4ml/converters/__init__.py", line 223, in convert_from_keras_model
    return keras_v3_to_hls(config)
  File "/work/NGT/hls4ml_enlupi/hls4ml/hls4ml/converters/keras_v3_to_hls.py", line 294, in keras_v3_to_hls
    return ModelGraph.from_layer_list(config, layer_list, input_layers, output_layers)
  File "/work/NGT/hls4ml_enlupi/hls4ml/hls4ml/model/graph.py", line 443, in from_layer_list
    model._make_graph(layer_list)
  File "/work/NGT/hls4ml_enlupi/hls4ml/hls4ml/model/graph.py", line 477, in _make_graph
    self.graph[name] = self.make_node(kind, name, layer, inputs, outputs)
  File "/work/NGT/hls4ml_enlupi/hls4ml/hls4ml/model/graph.py", line 566, in make_node
    node = layer_cls(self, name, attributes, inputs, outputs, initialize)
  File "/work/NGT/hls4ml_enlupi/hls4ml/hls4ml/model/layers.py", line 122, in __init__
    self.initialize()
  File "/work/NGT/hls4ml_enlupi/hls4ml/hls4ml/model/layers.py", line 1530, in initialize
    self.add_weights_variable(name=f'{dir}_weight', var_name=(f'w_{dir[0]}_' + '{index}'))
  File "/work/NGT/hls4ml_enlupi/hls4ml/hls4ml/model/layers.py", line 337, in add_weights_variable
    var = WeightVariable(
  File "/work/NGT/hls4ml_enlupi/hls4ml/hls4ml/model/types.py", line 562, in __init__
    self.shape = list(self.data.shape)
AttributeError: 'NoneType' object has no attribute 'shape'

Enrico Lupi and others added 22 commits June 12, 2025 16:35

ADD parsing for bidirectional RNN layers

f975066

Implement bidirectional rnn layers

8121281

ADD fixes

fc1e950

FIX resource strategy

f47bb5a

FIX infer precision for bidirectional rnn

c469793

FIX eliminate activation after bidirectional rnn

4c3d26e

FIX bidirectional layers name

5eef679

ADD tests for bidirectional layer

a9546c7

FIX weight name and ADD backward layer architecture check

0246dae

FIX static and non-static Bidirectional layers

7428af7

ADD parse general bidirectional layer with possibly different archite…

d2c6cc0

…ctures

ADD paring for general bidirectional layer

edf7cdf

ADD gnerale bidirectional wrapper

d882310

ADD Bidirectional layers support

4ed22c9

ADD support for reverse order layers

dd4f220

ADD feature check for merge mode and layers order

de803b7

ADD io type feature check

070fdc2

FIX n_out in case of merge_mode != concat

b65c730

ADD pytest for Bidirectional layer

b55cd04

FIX posible directions for LSTM and GRU

e8fae54

FIX spelling mistake

a1500e4

FIX order

1c16616

enlupi force-pushed the vivado_bidir_general branch from f929985 to 1c16616 Compare June 12, 2025 15:00

Enrico Lupi and others added 3 commits June 12, 2025 17:10

FIX remove unused import

2fc981c

FIX blank space

48f4fe2

Merge branch 'main' into vivado_bidir_general

64ab715

enlupi marked this pull request as ready for review June 23, 2025 12:16

JanFSchulte reviewed Jun 26, 2025

View reviewed changes

Merge branch 'main' into vivado_bidir_general

1b919c8

JanFSchulte added the please test Trigger testing by creating local PR branch label Jun 27, 2025

enlupi and others added 3 commits July 14, 2025 16:36

Merge branch 'fastmachinelearning:main' into vivado_bidir_general

20ff35e

RM old comments

734d42f

MV check for out-of-order layers from passes to parsing

7c128f7

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Bidirectional RNN layer support for Keras frontend and Vitis backend #1310

Bidirectional RNN layer support for Keras frontend and Vitis backend #1310

Uh oh!

enlupi commented Jun 12, 2025

Uh oh!

JanFSchulte Jun 26, 2025

Uh oh!

JanFSchulte Jun 26, 2025

Uh oh!

JanFSchulte Jun 26, 2025

Uh oh!

JanFSchulte Jun 26, 2025

Uh oh!

JanFSchulte Jun 26, 2025

Uh oh!

JanFSchulte Jun 26, 2025

Uh oh!

JanFSchulte Jun 26, 2025

Uh oh!

JanFSchulte Jun 26, 2025

Uh oh!

JanFSchulte commented Jun 26, 2025

Uh oh!

rimalroc commented Jun 27, 2025

Uh oh!

Uh oh!

Bidirectional RNN layer support for Keras frontend and Vitis backend #1310

Are you sure you want to change the base?

Bidirectional RNN layer support for Keras frontend and Vitis backend #1310

Uh oh!

Conversation

enlupi commented Jun 12, 2025

Description

Type of change

Tests

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JanFSchulte commented Jun 26, 2025

Uh oh!

rimalroc commented Jun 27, 2025

Uh oh!

Uh oh!