Closed hunse closed 4 years ago
I believe I changed it because, in general, you shouldn't modify global state in an import. I also think that before I reorganized some things, everything was being defined in the else
block of an if statement, which raised the C901 complexity warning, so I wanted to get rid of that and I think in making all those changes I moved some stuff to install_dl_builders
that was previously happening on import. It was easy to modify tests to work properly through calling install_dl_builders
explicitly, and in general we prefer explicit calls over automatic things that might make unexpected changes. But, given that you don't get an error due to those neuron types being subclasses, and it's not clear how you would introduce some kind of error, it's probably fine to call install_dl_builders
on import. Where would you add the call? In neurons.py
?
We could do it the main part of neurons.py
, or we could put it in the __init__
of each neuron type (that way nothing changes on import).
__init__
of the neuron type makes sense, though right now there's just one function for installing all builders, so that would have to be modified to allow for enabling one but not the other.
nengo_loihi.builder.nengo_dl
has the builders fornengo_dl
forLoihiLIF
andLoihiSpikingRectifiedLinear
.If you make a
nengo_loihi.Simulator
and tensorflow is installed, these builders will get properly initialized (see theinstall_dl_builders
) call inSimulator.__init__
. However, if you just import these neuron types fromnengo_loihi
to do training innengo_dl
, before you've created anengo_loihi.Simulator
, the builders won't be initialized.Even worse, these neuron types are subclasses of
LIF
andSpikingRectifiedLinear
thatnengo_dl
does know how to build, so it just ends up silently trying to build them but not working.I think that these builders should be initialized as soon as the neuron types are imported from
nengo_loihi.neurons
. Then, things will just work as people expect. This is what we had originally, but then we changed it for some reason, and I can't remember why. (@tbekolay, you're a co-author with me on that commit 025cdf997. Do you remember why?)If for some reason we can't guarantee that the builders will be initialized, then I think we need some way to throw an error if someone tries to use those neuron types in
nengo_dl
(and that error can point the user to how to initialize them).