Add IF_curr_delta model to PyNN namespace
Closed this issue · 7 comments
In response to requests from a user, I would like to add an IF_curr_delta model to PyNN.
The IFCurDelta model in spynnaker8.extra_models is a close match to the NEST iaf_psc_delta model. Please could you make IFCurDelta available from pyNN.spiNNaker under the name IF_curr_delta (for consistency with other PyNN standard models (even though it conflicts with PEP8))?
Our current testing tool do NOT allow us to add any code that conflicts with Pep8
To clarify we can of course exclude a few lines from our Pep8 tests but only do so in rare cases where the conversion to PEP8 standards would make the code significantly uglier.
For example docstrings with weird characters in them.
It is technically possible to exclude a whole file but as yet we have found not good justification of doing so.
Which is the specific PEP8 conflict you have and why can/should it not be fixed?
Ideally, "IF_curr_delta" should be "IFCurrDelta", but unfortunately none of the PyNN standard model names (like "IF_cond_exp") respect PEP8 ("Class names should normally use the CapWords convention")** and as PEP8 also says, "Consistency with this style guide is important. Consistency within a project is more important" - hence the class name needs to be "IF_curr_delta".
** (my excuse is that the model name convention was established back in 2007 when PEP8 was in general adhered to less rigorously)
I think it will be OK - we currently have no PEP8 issues with the existing names so it should be fine.
Keeping the names of different PyNN standard model consistent is a good case for having a Pep8 exception but as @rowleya says we do not appear to have any issues with the other PyNN standard model names,
To be clear is this request asking to expose https://github.com/SpiNNakerManchester/sPyNNaker/blob/master/spynnaker/pyNN/models/neuron/builds/if_curr_delta.py as pyNN.spiNNaker.IF_curr_delta in the same way that https://github.com/SpiNNakerManchester/sPyNNaker/blob/master/spynnaker/pyNN/models/neuron/builds/if_cond_exp_base.py is exposed as pyNN.spiNNaker.IF_curr_exp
If so our suggestion is to Do so but also leave it exposed as spynnaker8.extra_models
Bot with the same actual implementation of course so that any fixes to SpiNNakerManchester/sPyNNaker#762 appear in both
@Christian-B yes, that is what I am asking for
However, the version at pyNN.spiNNaker.IF_curr_delta should have a ΔV that is independent of the timestep, to match the behaviour of the NEST iaf_psc_delta model, so a separate implementation may be needed, depending on the outcome of SpiNNakerManchester/sPyNNaker#762