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

Merge progress report #139

Closed rgerkin closed 6 years ago

rgerkin commented 6 years ago

@russelljjarvis I think I'm close on getting the russell branch working, but I wanted to note a few things I've seen running test_exhaustive_search.py.
1) Your inject_square_current had a self.set_attrs(**self.attrs) call that was problematic because a) The backend (self) has no attrs (only the model does) and also because the only thing I could imagine putting there, is the model attrs, but then all that does is set the model attrs to themselves (since set_attrs is just a method for setting attrs of the model). So I removed that line, but I want to make sure you didn't have something else in mind that I missed.
2) The newest NEURON that I compiled on my computer has a sort of bug that prevents the Izhikevich model from being used. I edited NEURON and recompiled it to work around this, but there may still be other issues. I will check later against a Docker build that has an older NEURON. One NEURON error message that I am getting is:

NEURON: m_None_None_pop not an array variable
 near line 1
 m_None_None_pop[0].vpeak = 34.8935768891

2 Is, just NEURONs way of saying compiled *.mod files are not at a path they are expected to be in, it's not evidence of a bug in the NEURON installation. Also if you have jNEUROML working you also have a working version of NEURON, jNEUROml is not an alternative to NEURON, jNEUROml is just a way of hiding the NEURON implementation from the user.

3) I currently have a bunch of nrniv processes running but I'm not sure if they're really doing anything or just hanging. Nothing much has printed in my console, but I know that's at least in part because you suppressed a lot of print messages. stdout_worker is just showing the kinds of messages you see (for each worker) when you begin the NEURON simulation, e.g.:

engine.1.stdout: 
    Starting simulation in NEURON of 1600.0ms generated from NeuroML2 model...

Population RS_pop contains 1 instance(s) of component: RS of type: izhikevich2007Cell
Ignoring included LEMS file: Cells.xml
Ignoring included LEMS file: Networks.xml
Ignoring included LEMS file: Simulation.xml
Ignoring included LEMS file: Izh2007One.net.nml

What should I expect to see in the main console and in stdout_worker output if things are working?

rgerkin commented 6 years ago

I should also note that even the simple jnml LEMS_2007One.xml -neuron -run finishes a simulation with:

(INFO) NRN Output >>> Population RS_pop contains 1 instance(s) of component: RS of type: izhikevich2007Cell
(INFO) NRN Output >>> Running a simulation of 520.0ms (dt = 0.0025ms; seed=123456789)
(INFO) NRN Output >>> Finished NEURON simulation in 0.729698 seconds (0.012162 mins)...
(INFO) NRN Output >>> Saving results at t=520.0000000055478...
(INFO) NRN Output >>> Saved data to: time.dat
(INFO) NRN Output >>> first instance of n_RS_pop

but then hangs, so there may be something messed up about my version of NEURON (I don't think it is Mac related, just related to new version of NEURON I compiled).

russelljjarvis commented 6 years ago

Is this NEURON-7.5 ? Can you plot a $ V_{m} $ with this new version of NEURON?

Given that we are only using embarrassingly parallel python/NEURON, I don't think we actually have to build NEURON from source. Also I believe pyNN's pip install ships with a copy of NEURON too (although when using it, it is still easy to become plagued by the uncompiled *.mod file problem, however Problem 2 is my fault.

Ordainarily, the std_out from 2, is just NEURONs way of saying compiled *.mod files are not at a path they are expected to be in, it's not evidence of a bug in the NEURON installation, however, this the reason no NEURON mechanisms are sense is caused by the cell_name containing: None_None, (eg. NEURON: m_None_None_pop not an array variable) which is a consequence of my latest code changes. I believed it was tested but apparently it is not.

NEURON: m_None_None_pop not an array variable
 near line 1
 m_None_None_pop[0].vpeak = 34.8935768891

The relevant lines are at: https://github.com/russelljjarvis/fdcheck/blob/test_branch/neuronunit/models/backends.py#L372

https://github.com/russelljjarvis/fdcheck/blob/test_branch/neuronunit/models/backends.py#L165-1687

can just delete all references in the constructor to

self.cell_name = None

and

self.current_source_name = None

This was just my attempt at stopping those jneuroml file reads that eminate in the stdout for example Ignoring LEMS file *.xml etc.

Also I wonder if you would be able to paste in a line into test_exhaustive_search.py import numpy as np at the top. The code fails to execute a report because I forgot to include numpy import.

rgerkin commented 6 years ago

Yes

rgerkin commented 6 years ago

Actually, I'm having the same problem with NEURON 7.4 (just compiled from source), i.e. where jnml LEMS_2007One.xml -neuron -run finishes but then hangs.

rgerkin commented 6 years ago

I think I figured it out. First, for some reason the generated python file is trying to write results into a results directory that does not exist, so I just created an empty one. This is also solved by editing the LEMS file, which is something I think I did a long time ago (we have been using that one for a long time), but it returned when I updated to the new version of the model. So I fixed it again.

The command line jnml command was hanging because I didn't use -nogui, so it was waiting for the X server. I added -nogui and that went away. I should be able to make further progress from here.

rgerkin commented 6 years ago

OK, it's running now (so far), and here's what I did:

When I stopped setting cell_name and current_src_name to None, it still had problems because other parts of the code expected these to be set, and they are otherwise only set in the more_attributes section at the end of load_model. But I think there are other places in the code that are requesting them before load_model is called, which is probably why you wanted to set them directly using arguments to init_backend.

So I added these property attributes to the Backend:

@property
    def cell_name(self):
        return getattr(self,'_cell_name','RS')

    @property
    def current_src_name(self):
        return getattr(self,'_current_src_name','RS')

and changed it so that the more attributes section sets the underscored versions of these variables instead of the non-underscored ones. This way, the default value of both self.cell_name and self.current_src_name is "RS" (a dirty hack we will have to fix next week), but as soon as more attributes gets run, these value will change to whatever more attributes sets them to.

russelljjarvis commented 6 years ago

These latest changes look great.

Sorry for my part in making this a difficult merge for you. I have been thinking a lot about continuous integration and git commits.

rgerkin commented 6 years ago

The parallel rheobase test now runs if used to test one model one time (in a notebook). The problem I had before was that when I started using the new Izhikevich model files, which use an "include" directive to include most of the important parameters, your more_attributes section was not successfully reading them, because the pynml.read_lems_file call doesn't read included files by default. I made some changes to pyNeuroML and pyLEMS to allow it to read them. We will probably have to use our own forks of both of those for a while.

It still hangs when running it in test_exhaustive_search, but I'll get to the bottom of that this week.

russelljjarvis commented 6 years ago

I recommend using the following travis build is a useful reference point, as it shows the execution of the code of test_exhaustive_search including nsga_parallel.py without hanging: (the build failure is a timeout, which is reasonable because the run involves both grid and GA optimization).

Also slightly before I created fdcheck, test_branch was also passing exhaustive_search.py too: https://travis-ci.org/russelljjarvis/neuronunit/builds/298314871#L7790-7817 So the if you wind back two commits on russelljjarvis:test_branch exhaustive_search that could be a slightly less good reference point.

The following are code hotspots, changes to which I associate with the code no longer hanging. Previously the method pre_format was parallel, but it is not a significant overhead, and I associate the serial mapping with not the code not hanging also. https://github.com/russelljjarvis/fdcheck/blob/test_branch/neuronunit/optimization/nsga_parallel.py#L139-L160

json/pickle dumps here: https://github.com/russelljjarvis/neuronunit/blob/test_branch/neuronunit/neuroelectro.py

https://github.com/russelljjarvis/neuronunit/blob/test_branch/neuronunit/optimization/get_neab.py#76-77

Also let me know if there is a remote that I can use to do a graphical merge with on my end. I really like this GUI Meld, its great for threeway graphical merge on OSX and Linux. You can invoke it with git mergetool --tool=meld provided it is already installed. Installation is straightforward if you have XQuartz. I usually merge on a host machine rather than through the GH pull request itself, but I think I remember you were saying that you wanted to avoid that process.

rgerkin commented 6 years ago

I've gotten pretty far now, but am now getting this error:

Traceback (most recent call last):
  File "test_exhaustive_search.py", line 80, in <module>
    quantize_distance = list(np.linspace(minimagr,maximagr,10))
  File "/home/rgerkin/miniconda3/lib/python3.6/site-packages/numpy/core/function_base.py", line 108, in linspace
    start = asanyarray(start) * 1.0
TypeError: unsupported operand type(s) for *: 'DataTC' and 'float'

I will look into it further tomorrow unless you have any thoughts before then.

russelljjarvis commented 6 years ago

AOK. The general idea is this:

quantize distance between minimimum error and maximum error.

quantize_distance = list(np.linspace(mini,maxi,10)) #4 

In that version of the code, I had forgotten that min_find returns the DTC with the lowest score, and not the lowest score itself.

rgerkin commented 6 years ago

OK, it all runs now (with that and a few other small fixes). However, I would like to check the output I am getting against the output that you are getting. Could you paste here any output (not the messages from running the simulations, but all the output from all the checks that begin with def min_find. BTW, this is on my Mac, so that is a good sign.

rgerkin commented 6 years ago

I pushed my version here so you can see the output.