Added support for keras RNN #329

chaitanyaPaikara · 2021-04-29T22:48:28Z

Working keras-converter and RNN template code for LSTM layer, will update with GRU code later. Review @vloncar

thesps · 2021-05-05T10:10:22Z

hls4ml/templates/vivado_template.py

+rnn_mult1_config_template = """struct config{index}_mult1 : nnet::dense_config {{
+    static const unsigned n_in = {n_in};
+    static const unsigned n_out = {n_out};
+    static const unsigned io_type = nnet::{iotype};
+    static const unsigned strategy = nnet::{strategy};
+    static const unsigned reuse_factor = {reuse};
+    typedef {accum_t} accum_t;
+    typedef {bias_t} bias_t;
+    typedef {weight_t} weight_t;
+    template<class x_T, class y_T, class res_T>
+    using product = nnet::product::{product_type}<x_T, y_T, res_T>;
+}};\n"""
+
+
+rnn_mult2_config_template = """struct config{index}_mult2 : nnet::dense_config {{
+    static const unsigned n_in = {n_in};
+    static const unsigned n_out = {n_out};
+    static const unsigned io_type = nnet::{iotype};
+    static const unsigned strategy = nnet::{strategy};
+    static const unsigned reuse_factor = {reuse};
+    typedef {accum_t} accum_t;
+    typedef {bias_t} bias_t;
+    typedef {weight_t} weight_t;
+    template<class x_T, class y_T, class res_T>
+    using product = nnet::product::{product_type}<x_T, y_T, res_T>;
+}};\n"""
+


Since the contents of these are the same as the Dense config, do we need to define new templates? Can you just pick up the existing Dense template?

Reusing the Dense config templates and removed theses new definitions of mult configs

thesps · 2021-05-05T10:19:35Z

hls4ml/templates/vivado/nnet_utils/nnet_common.h

+// Activation enum
+enum activ_type {activ_relu = 0, activ_sigmoid, activ_tanh, activ_softmax};
+


I think we should find a nicer mechanism to enable multiple activation functions from the RNN. One method could be to make classes for the activation functions, and then include in the RNN layer config the object that corresponds to the appropriate activation.

The aim would be to eliminate the if (CONFIG_T::activation_type == activation_relu) { relu<...>(...) } constructions in the HLS and replace it with a single line like:
CONFIG_T::template activation<...>::activation(...)

It's similar to what we do now with the nnet::mult - classes here, config here, and call to product here.

Followed the same method used for nnet::mult and passing objects for appropriate activation now.

thesps · 2021-05-05T10:29:37Z

hls4ml/templates/vivado/nnet_utils/nnet_recurrent.h

+//      - output   = V*newstate
+//  - If softmax is needed on output, perform *outside* this operations
+template<class data_T, class res_T, typename CONFIG_T, typename ACT_CONFIG_T>
+void simple_rnn(


It looks like these simple_rnn and simple_rnn_static methods are not accessible through the hls4ml conversion, only the LSTM and GRU. Is that correct? If so, these implementations should be removed. Or otherwise, they should be made accessible through the conversion flow.

Yeah, we can remove them as people hardly use basic RNN layers.

vloncar

Thanks for updating the previous PR to the latest code! This is a great start

vloncar · 2021-05-05T13:23:32Z

hls4ml/converters/keras/recurrent.py

+    layer['n_out'] = int(weights_shape[1]/div_factor)
+    layer['recurr_n_in']=recurrent_weights_shape[0]
+    layer['recurr_n_out']=recurrent_weights_shape[1]
+    layer['recurr_n_subout']=[recurrent_weights_shape[1]]


There are still remnants of sublayer behavior, they should be removed since we abandoned that idea.

vloncar · 2021-05-05T13:31:35Z

hls4ml/converters/keras/recurrent.py

+
+    layer = parse_default_keras_layer(keras_layer, input_names)
+
+    weights_shape = data_reader.get_weights_shape(layer['name'], 'kernel')


Instead of inferring attributes from weights, we should extract the number of hidden units and use that to compute the shapes. Keras uses this (units), as well as pytorch and onnx (as hidden_size attribute).

Also, other LSTM layer properties should be parsed as well. See the docs for all things that can be set. Even if we don't support them all in the current HLS implementation we should be aware of them so we can provide a message to the user if something is not supported as opposed to silently ignoring them and doing something unintended.

I followed the same format used for other layers which were using weight_shape to figure out the shape. I've added all the architecture related properties of the RNN layer to the layer dictionary. Only properties associated with training aren't added.

What about adding the parsing of all the other layer properties, could you add that @chaitanyaPaikara ?

@thesps, Yeah, I can add those other properties but they were all properties related to model training, so I skipped them as they were not influencing the inference implementation with hls4ml templates.

return_state, go_backwards, stateful, time_major, reset_after... Preferably the conversion should check the unsupported parameters in initialize and raise exceptions in case of configuration we can't handle yet.

vloncar · 2021-05-05T13:33:20Z

hls4ml/converters/keras/recurrent.py

+    layer['recurr_n_subout']=[recurrent_weights_shape[1]]
+    layer['recurr_n_part'] = 1
+
+    output_shape = [input_shapes[0][0], layer['n_sequence_out'], layer['n_out']]


If return_sequences is set to False, which is the default, the output shape is 2D, not 3D. This will cause the issues when parsing the architecture.

vloncar · 2021-05-05T13:34:49Z

hls4ml/converters/keras_to_hls.py

@@ -118,6 +118,8 @@ def parse_default_keras_layer(keras_layer, input_names):
        layer['activation'] = keras_layer['config']['activation']
    if 'epsilon' in keras_layer['config']:
        layer['epsilon'] = keras_layer['config']['epsilon']
+    if 'recurrent_activation' in keras_layer['config']:


Activations are separated out here because in hls4ml we do them separately from the layer in which they are defined. Since that is not the case for RNNs, recurrent_activation should not be parsed here, but rather in the appropriate handler.

vloncar · 2021-05-05T13:36:39Z

hls4ml/model/hls_layers.py

+        if self.model.config.is_resource_strategy(self):
+            if self.model.config.backend.name == 'Vivado':
+                self.model.config.backend.set_closest_reuse_factor(self)
+                self.model.config.backend.set_closest_reuse_factor_recr(self)


Since this function changes the object passed to it and doesn't return values that are assigned to an object, it would be better to extend set_closest_reuse_factor and not introduce the _recr variant and add recr parameter to it.

vloncar · 2021-05-05T13:44:03Z

hls4ml/templates/vivado_template.py

-    def get_valid_reuse_factors(self, layer):
+        self.register_templates('GarNetStack'            , garnet_stack_function_template,garnet_stack_config_template, garnet_include_list)    
+
+    def get_valid_reuse_factors(self, layer, recr = False):


I mentioned this before, I think you can get away without recr parameter. Or refactor this function so that it accepts n_in and n_out parameters and move their computation to the caller

I have extended the set_closest_reuse_factor to also check for the recr and I think its more straightforward or simple to check for recr parameter and find range of reuse factors. Also, a recr is essential in the set_closest_reuse_factor as there are two matrix multiplications for regular and recurrent weights, and passing to get_closest_reuse_factor just makes it easier without much code change in the original file. If necessary, I can still refactor it to do away with the recr.

vloncar · 2021-05-05T13:46:00Z

hls4ml/templates/vivado/nnet_utils/nnet_recurrent.h

+        tanh<typename CONFIG_T::state_t, typename CONFIG_T::state_t, ACT_CONFIG_T>(rawstate, newstate);
+    }
+
+    std::cout << "Activated State: [ "; for (int ii = 0; ii < CONFIG_T::n_state; ii++) std::cout << newstate[ii] << " "; std::cout << "]" << std::endl;


This is not useful unless we are debugging the implementation itself so it should be removed.

This was part of the basic RNN layer. Will be removing those functions as they are not used anyway.

vloncar · 2021-05-05T13:47:10Z

hls4ml/templates/vivado/nnet_utils/nnet_recurrent.h

+	    ) {
+  // Initialize the state variable -- will maintain state between function calls
+  # ifndef __SYNTHESIS__
+  std::cout << "S Pre-State: [ "; for (int ii = 0; ii < CONFIG_T::n_state; ii++) std::cout << s_newstate[ii] << " "; std::cout << "]" << std::endl;


Same as above

vloncar · 2021-05-05T13:50:50Z

hls4ml/templates/vivado/nnet_utils/nnet_recurrent.h

+}
+
+template<class data_T, class res_T, typename CONFIG_T>
+  void lstm_loop(


I would prefer if this was called just lstm or lstm_stack and then the loop iteration should be lstm_loop

changed to lstm_stack

vloncar · 2021-05-05T13:51:42Z

hls4ml/templates/vivado/nnet_utils/nnet_recurrent.h

+}
+
+template<class data_T, class res_T, typename CONFIG_T>
+  void gru_loop(


Same for this

changed to gru_stack

thesps · 2021-05-11T08:27:34Z

Hi @chaitanyaPaikara, let's only mark review items as "resolved" when the code changes that address the issue are pushed please.

…ons; and removed redundant functions and template code

chaitanyaPaikara · 2021-05-12T22:38:34Z

Hi @thesps and @vloncar, I've pushed the latest changes addressing the above issues. Please let me know once you've reviewed these changes.

chaitanyaPaikara · 2021-05-29T21:46:10Z

Hi @vloncar and @thesps, could you please review the added changes and let me know if there are other issues.

vloncar · 2022-07-21T17:15:56Z

Done in #560. Closing

chaitanyaPaikara added 2 commits April 26, 2021 17:19

Support for LSTM layers

2246ccb

Corrected trailing tabs and spaces

28ee195

thesps reviewed May 5, 2021

View reviewed changes

Adding io_stream support for GRU layer and N_subout correction

f263ddd

vloncar requested changes May 5, 2021

View reviewed changes

Fixed weight transpose issue for the recurrent weights

673c085

Modified the conversion flow to handle objects for appropriate functi…

8132481

…ons; and removed redundant functions and template code

Correcting typo in GRU template code

062008a

This was referenced Aug 12, 2021

Recursive2 #89

Closed

Keras LSTM and GRU implementation #193

Closed

jmduarte mentioned this pull request Sep 10, 2021

RNN/LSTM in HLS Library #13

Closed

vloncar closed this Jul 21, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added support for keras RNN #329

Added support for keras RNN #329

chaitanyaPaikara commented Apr 29, 2021 •

edited

Loading

thesps May 5, 2021

chaitanyaPaikara May 9, 2021

thesps May 5, 2021

chaitanyaPaikara May 12, 2021

thesps May 5, 2021

chaitanyaPaikara May 9, 2021

vloncar left a comment

vloncar May 5, 2021 •

edited

Loading

vloncar May 5, 2021

chaitanyaPaikara May 12, 2021

thesps May 31, 2021

chaitanyaPaikara Jun 1, 2021

vloncar Jun 2, 2021

vloncar May 5, 2021

vloncar May 5, 2021

vloncar May 5, 2021

vloncar May 5, 2021

chaitanyaPaikara May 11, 2021

vloncar May 5, 2021

chaitanyaPaikara May 9, 2021

vloncar May 5, 2021

vloncar May 5, 2021

chaitanyaPaikara May 9, 2021

vloncar May 5, 2021

chaitanyaPaikara May 9, 2021

thesps commented May 11, 2021

chaitanyaPaikara commented May 12, 2021

chaitanyaPaikara commented May 29, 2021

vloncar commented Jul 21, 2022

		// Activation enum
		enum activ_type {activ_relu = 0, activ_sigmoid, activ_tanh, activ_softmax};


		layer = parse_default_keras_layer(keras_layer, input_names)

		weights_shape = data_reader.get_weights_shape(layer['name'], 'kernel')

Added support for keras RNN #329

Added support for keras RNN #329

Conversation

chaitanyaPaikara commented Apr 29, 2021 • edited Loading

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

vloncar left a comment

Choose a reason for hiding this comment

vloncar May 5, 2021 • edited Loading

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

Choose a reason for hiding this comment

thesps commented May 11, 2021

chaitanyaPaikara commented May 12, 2021

chaitanyaPaikara commented May 29, 2021

vloncar commented Jul 21, 2022

chaitanyaPaikara commented Apr 29, 2021 •

edited

Loading

vloncar May 5, 2021 •

edited

Loading