Vivado LSTM / GRU Bugs - PR #576

[bo3z]

Prerequisites

Please make sure to check off these prerequisites before submitting a bug report.

• Test that the bug appears on the current version of the master branch. Make sure to include the commit hash of the commit you checked out.
• Check that the issue hasn't already been reported, by checking the currently open issues.
• If there are steps to reproduce the problem, make sure to write them down below.
• If relevant, please include the hls4ml project files, which were created directly before and/or after the bug.

Quick summary

The recently merged PR #576 which added support for LSTM/GRU layers on Vivado has the following issues:

• Fails to compile - Minor errors, possible solution on this branch
• GRU Incorrect output for non-zero bias - test cases were included to reproduce this behaviour on the above mentioned branch. Alternatively, the code below will reproduce the error as well.

Details

• Both GRU & LSTM fail to compile with the current master branch of hls4ml, commit link. This occurs due to an incorrect template instantiation of nnet::product and incorrect function call for streaming LSTM. Both have been addressed on this branch

• Incorrect GRU output for non-zero bias - when using a random bias initialiser for GRU, the error, when compared to Keras, is quite significant.

Steps to Reproduce

1. Clone the hls4ml repository
2. Checkout the master branch, with commit hash: 59ed824
3. Run conversion on model and verify its output file with code (note, a similar approach should be done with LSTM, as well as io_stream):

input_shape = (5, 8)
X = np.random.rand(50, *input_shape) 

keras_model = Sequential()
keras_model.add(GRU(units=32, input_shape=input_shape, kernel_initializer='lecun_uniform', bias_initializer='lecun_uniform'))
keras_model.compile()

default_precision = 'ap_fixed<32, 16>'
hls_config = hls4ml.utils.config_from_keras_model(keras_model, granularity='name', default_precision=default_precision)
output_dir = 'gru_test'

hls_model = hls4ml.converters.convert_from_keras_model(
                        keras_model, 
                        hls_config=hls_config,
                        output_dir=output_dir, 
                        backend='Vivado',
                        io_type='io_parallel'
)
hls_model.compile()

keras_prediction = keras_model.predict(X)
hls_prediction = hls_model.predict(X)
np.testing.assert_allclose(hls_prediction.flatten(), keras_prediction.flatten(), rtol=0.0, atol=3e-2)

Expected behaviour

• Model compiles successfully
• Output is approximately equal to Keras output

Actual behaviour

• Model fails to compile when using master branch
• Once compilation issues are fixed, most layers perform correctly, except for GRU with non-zero behaviour

[dsrankin]

@bo3z Thanks a lot for this, not quite sure why my original PR didn't seem to have these template issues, but in any case, this does look to fix most of the problems! And thanks for spotting the issue with the stream version, I hadn't tested those myself. I'm looking into the non-zero GRU bias issue now...

[bo3z]

@drankincms I encountered the same problem when implementing GRUs on Quartus. Turns out the weights from Keras aren't parsed correctly - a GRU unit in Keras has both bias and recurrent bias. The current implementation simply extracts the bias and sets the recurrent bias to zero, allowing for matrix multiplication using Dense. Instead, in Keras, the bias array, is a 2-dimensional array of arrays, in which the first array has 3 * n_states elements, those being biases for z, r, and h_candidate. Similarly, the second array is of equal size, but for recurrent biases. This can be verified by the following command: print(keras_model.layers[0].get_weights()[2]). I believe that this was a recent change, because in this blog post, Keras uses a a single bias: https://dieuwkehupkes.nl/finding-recurrent-network-weights-in-keras-models/.

A possible fix is attached in the screenshot below:

[dsrankin]

@bo3z Great catch, thanks for spotting this and sending the fix over! I added this to the branch you made, things look to be working properly now. Let me know if you want to make any other edits before we push the changes to master/main.

[bo3z]

That's great, I reckon this can be merged with master now.

[bo3z]

Resolved.