Add LayerNorm support for Vivado #1110
Conversation
|
The pytest failure is the QKeras tests timing out. There's 299 tests being run in that batch, which I guess is too many. Is there a why to reshuffle the batches to avoid the timeout? |
|
pre-commit.ci autofix |
jmitrevs
added
please test
|
For the "broken" diffs, we should look to see what the whitespace error is and fix it before merging. |
|
I did a bit of digging and it's not a whitespace problem but rather the file endings are improperly encoded. Likely the person we got the code from was using a Windows machine. @rianbrooksflynn, you can install the |
JanFSchulte
added
please test
|
We should squash the commits when we merge this. |
| static const unsigned n_in = {n_in}; | ||
| static const unsigned seq_len = {seq_len}; | ||
| static const unsigned table_size = {table_size}; | ||
| static constexpr double table_range = {table_range}; |
There was a problem hiding this comment.
Any reason this is a double? It is not used as such, and breaks Vivado synthesis.
| static const unsigned io_type = nnet::{iotype}; | ||
| static const unsigned reuse_factor = {reuse}; | ||
| static const bool store_weights_in_bram = false; | ||
| static constexpr double epsilon = {epsilon}; |
There was a problem hiding this comment.
Similar to above, this will only ever be used to store values in table_t, so we should ensure epsilon is compatible
| 'table_t', NamedType(name=layer.name + '_table_t', precision=FixedPrecisionType(width=16, integer=6)) | ||
| ) | ||
| if 'table_size' not in layer.attributes: | ||
| layer.set_attr('table_size', 4096) # table size |
There was a problem hiding this comment.
These attributes should be set as default in _register_layer_attributes() not here.
Also, is 4096 necessary for this implementation to work? All other tables are 1024.
| if layer['epsilon'] <= 0: | ||
| raise Exception('epsilon must be positive') | ||
|
|
||
| return layer, [shape for shape in input_shapes[0]] |
There was a problem hiding this comment.
This function should also parse axis parameter to avoid misparsing. Sure, we only support axis=-1 and we can raise exceptions about it, but should be handled.
| layer['n_in'] = layer['n_out'] = in_size | ||
|
|
||
| if not ((len(input_shapes[0])) == 3): | ||
| raise Exception('input size is not currently supported by hls4ml, only dim3 is supported') |
There was a problem hiding this comment.
What is dim3? This sentence could be made a bit nicer :-)
| _expected_attributes = [ | ||
| Attribute('n_in'), | ||
| Attribute('seq_len'), | ||
| Attribute('epsilon', value_type=float, default=1e-3), |
| // Resource reuse info | ||
| static const unsigned io_type = io_parallel; | ||
| static const unsigned reuse_factor = 1; | ||
| static const bool store_weights_in_bram = false; |
There was a problem hiding this comment.
Can we tidy up this example config so that it has the fields that are actually expected (like all types and ranges) and not the ones that are not used (like weights_in_bram)
| @pytest.fixture(scope='module') | ||
| def model(): | ||
| model = Sequential() | ||
| model.add(LayerNormalization(input_shape=in_shape)) |
There was a problem hiding this comment.
Can we do a more complex configuration that ensures we actually parsed correctly and not just used default values from hls4ml/Keras?
| # Currently only Vivado/Vitis in io_parallel mode is supported | ||
| @pytest.mark.parametrize('backend', ['Vivado', 'Vitis']) | ||
| def test_layernorm(model, data, backend): | ||
| config = hls4ml.utils.config_from_keras_model(model, granularity='name', backend=backend) |
There was a problem hiding this comment.
Can we tweak the config to override ranges and table sizes to validate backend is picking them up correctly?
|
|
||
| @pytest.fixture(scope='module') | ||
| def model(): | ||
| model = nn.Sequential(nn.LayerNorm(in_shape[-1])) |
There was a problem hiding this comment.
Can we do a more complex configuration that ensures we actually parsed correctly and not just used default values from hls4ml/PyTorch?
Description
This PR adds support for Layer Normalization using either Keras or PyTorch with the Vivado backend in
io_parallelmode.This implementation uses a lookup table for inverse square root; the inputs to the lookup table follow a logarithmic distribution for better accuracy.
Tests have been added for both Keras and Pytorch parsing.
Credit is due to @Ethan0Jiang and @LostEcho365 (Zhixing Jiang and Dennis Yin) for their Vivado implementation and Keras parsing support; my contributions were making a change to the inverse square root lookup table implementation, implementing PyTorch support, and adding unit tests. (Here's a link to their pre-print.) The original code authors have given permission for their code to be merged into hls4ml.
Linked issue: #1109
Type of change
Tests
Two unit tests added:
test/pytest/test_layernorm.pyandtest/pytest/test_layernorm_pytorch.pyChecklist
pre-commiton the files I edited or added.