scidash / neuronunit

A package for data-driven validation of neuron and ion channel models using SciUnit
http://neuronunit.scidash.org
38 stars 24 forks source link

broken dev build #201

Closed JustasB closed 5 years ago

JustasB commented 5 years ago

@russelljjarvis I noticed the latest build on dev is not working. Will you check in a working version?

I'm working on morphology unit tests and I want to be confident my tests will work with the latest NU version -- but the unit tests are failing.

russelljjarvis commented 5 years ago

There is a bit of a responsibility conflict with that branch at the moment. Rick was performing, a merge into dev, and we wanted to isolate my development to facilitate his merge.

Meanwhile, I have started the process of creating more meaningful unit tests, in the more recently maintained branch opt_cluster, where I check the effectiveness of optimization, against all the different backends. It's probably about time, I merged opt_cluster, into dev, but that will temporarily break things even more (for about 24 hours at least, then things could be better).

It looked like test_exhaustive_search.py was using numpy as np before it was imported. When the merge is complete, the file test_exhaustive_search will be superseded by test_ga_versus_grid.py. I have embarked on this process now.

rgerkin commented 5 years ago

All tests passed 3 commits earlier, then 2 commits earlier, Python 2.7 failed but others were still passing. Those reflected my commits. Then Russell made minor changes that could not possibly have caused the current failures, and yet the tests are now failing. So my guess is that it is something upstream, maybe SciUnit. I will test on my machine.

rgerkin commented 5 years ago

Fixed in dfdaa1a1d04509eba067af5f9867196b5bb2ebf2.

Previously I had changed some tests to set attributes like current amplitudes on construction, rather than as class attributes. This was done because test instances run in parallel might need different current amplitudes. @russelljjarvis solved this his own way in his fork, and when I pulled his changes in I refactored to put this solution into the parent class for these tests.

But RestingPotentialTest got left out of this, presumably out of confusion about whether such a test should even have a current. I decided (a long time ago) that it should, and that the current should be 0, otherwise existing external currents in the model will be applied by default, and it isn't a resting potential anymore. So in the last commit I applied the same thing to resting potential, but for 0 current (always). That fixes the unit tests.

JustasB commented 5 years ago

Thank you @rgerkin

russelljjarvis commented 5 years ago

Okay I did a merge between scidash/dev, and russelljarvis/dev. I expect that, that merge breaks all of my unit_tests, which now live in test_low_level.py and test_high_level.py

https://github.com/russelljjarvis/neuronunit/commit/f8fc6d5ad6b26c6c59e7b3578b6ff63d5a66e933