Basic SNN functionality in hls4ml (mapped from snntorch)#1470
Basic SNN functionality in hls4ml (mapped from snntorch)#1470bmdillon wants to merge 33 commits into
Conversation
|
pre-commit.ci autofix |
vloncar
left a comment
There was a problem hiding this comment.
Overall looks good, needs minor changes to keep it in line with the rest of the codebase
|
pre-commit.ci autofix |
| for node in model.graph.values(): | ||
| if isinstance(node, (IFNeuron, LIFNeuron)) and node.get_attr('window_size') != window_size: | ||
| node.set_attr('window_size', window_size) | ||
| changed = True |
There was a problem hiding this comment.
I don't think this is needed. It is required when changes are made to the execution graph (nodes added/removed/replaced), or if change in an attribute of one node affects some other. But the latter case should be avoided. I don't think that we need to return True in this pass. Can you check if it works without it?
There was a problem hiding this comment.
This is needed to update the window_size in the spiking neurons. Everything is mapped from snntorch, and in snntorch the Leaky neurons do not have a window_size parameter, so hls4ml defaults it to zero, and then updates it later to match the window_size in the readout layer. Without it, the window_size parameter in the spiking neurons stays at 0 and would not reset properly. I ran a test locally to confirm this is needed.
| else: | ||
| if attr.default is not None: | ||
| if attr.name in layer: | ||
| layer_config[attr.config_name] = layer[attr.name] |
There was a problem hiding this comment.
So the idea here is that there may be attributes of the layer that the user configured during model definition, which would then be part of the model when parsed, but that the user can still override during conversion and deployment? Up to this point we considered the semantics of configurable attributes as something purely in hls4ml that controls the behavior of the model in a way that cannot be specified during model definition (in e.g., pytorch, snntorch or kersas), such as the reuse factor or type definitions for non-quantized models. One can argue this breaks the concept of what it means for something to be configurable, but I don't have strong opinions on enforcing it. We kept it hidden (not part of the dict this method produces, but still doable if you really know what you're doing). Thoughts @JanFSchulte @jmitrevs @calad0i @bo3z ?
There was a problem hiding this comment.
This looks good to me. Maybe apply the same change to the other frontends also for consistency.
I would argue that the qkeras autoconfig generator is already doing similar things with the quantizers, i.e., quantizer params defined through config and overridable by the user, though not the same Attribute knobs, I would argue they are similar.
If the user overrides anything inside the default config, they shall know what they are doing. Also, adding the
"default" config here don't change the actual downstream behavior (if this doesn't exist, one can still insert it manually), so also no new footgun.
There was a problem hiding this comment.
Same change now applied in keras and onnx frontends in config.py
|
To me this looks ready, assuming tests pass. Let's see if other main devs have issue with the slight change in semantics of configurable attributes. |


Description
This PR adds initial PyTorch/SNN support to hls4ml.
It includes conversion support for IF/LIF neuron layers, SNN readout handling, Vivado templates, documentation, tests, and a small example notebook.
Dependencies:
snntorchtorchType of change
Tests
Added pytest coverage for PyTorch SNN parsing, IF/LIF layer handling, scalar/vector beta and threshold values, SNN readout options, and custom PyTorch SNN extension parsing.