Bug re-introduced with merge in pull request. about neuronunit HOT 7 CLOSED

@russelljjarvis
Are you just noting that you corrected it, or do you want me to correct it?

from neuronunit.

Both. I corrected it my branch and remote, but it may still exist in scidash.

from neuronunit.

@russelljjarvis
i.e. the fixed version is in the russell_dev branch but not the dev branch at scidash?

from neuronunit.

Oh I see you are trying to get a reference to where it is fixed in code, where as I have only given reference to where it is broken.
To be clear its broken on scidash /dev, and master:
https://github.com/scidash/neuronunit/blob/dev/neuronunit/tests/__init__.py#L215
https://github.com/scidash/neuronunit/blob/master/neuronunit/tests/__init__.py#L180

Its only reverted to my old fix in the following remote as of presently:

https://github.com/russelljjarvis/neuronunit/blob/dev/neuronunit/tests/__init__.py#L215

from neuronunit.

@russelljjarvis I looked more closely at this and there are some other bugs. For example, it makes no sense that y is being multiplied by pq.ms because y is a voltage.

So I've developed a recipe for making sure that the time constant is calculated correctly, shown here. If you replace the methods in TestPulseTest with the corresponding methods in Dummy then it should work. Try it out and see if it gives sensible results for your application.

If it does, then I can go back and eliminate Dummy from that notebook and just instantiate TimeConstantTest instead, and the notebook will serve as a unit test for the functionality of TimeConstantTest.

from neuronunit.

Sounds good. Some of my fixes are based on satisfying syntax requirements without much understanding of the purpose of the function and what it is trying to solve. So my first step will probably involve using ipython notebook convert, to wrangle the json into regular ascii encoded .py file. I like it that there is a unit test directory now.

from neuronunit.

I think this is not a bug. However, the code has also been substantially revised and I don't think this problem still exists, at least not in its current form. You can see that there is a version of this which is working with correct units, which I documented as part of 02d0a37.

from neuronunit.