neuromorphs / NIRTorch

PyTorch helper module to translate to and from NIR
https://neuroir.org/docs
BSD 3-Clause "New" or "Revised" License
7 stars 3 forks source link

expose ignore_submodules in extract_nir_graph #12

Closed stevenabreu7 closed 1 year ago

stevenabreu7 commented 1 year ago

to support RNN models in snnTorch, we can simply ignore all submodules within the RLeaky or RSynaptic modules, since we will handle them manually directly in snnTorch.

We then use NIRTorch as follows from within snnTorch to extract a NIR graph:

return extract_nir_graph(
        module, _extract_snntorch_module, sample_data, model_name=model_name,
        ignore_submodules_of=[RLeaky, RSynaptic]
    )
Jegp commented 1 year ago

It makes sense to want to ignore submodules, but I wonder if this is the right approach. Wouldn't the following logic make more sense?

if map_exists_for_module(m):
   try_convert_module(m)
elif module_is_recursive(m):
   continue_recursively(m.children())

In other words, would it make more sense in nirtorch to check if the module has been converted, and if so, don't touch the submodules of that module?

Separate to that, I wonder whether the ignore_submodules_of parameter might still be good to have

stevenabreu7 commented 1 year ago

That would be cleaner, I agree! But actually, the problem is that the submodule is already added and then the get_root method returns multiple nodes (the submodule nodes don't have any incoming nodes), so I will just keep it as it is for now - but let's change it to something cleaner later