Possible off-by-one bug in ModelView > Export to NeuroML

Suggestions for improvements to the NEURON Forum.
Post Reply
JustasB
Posts: 28
Joined: Tue Dec 08, 2015 8:17 pm
Location: Tempe, AZ, USA
Contact:

Possible off-by-one bug in ModelView > Export to NeuroML

Post by JustasB » Wed Mar 20, 2019 9:48 pm

I'm running into an issue where when I try to export a model to NeuroML, it generates an XML file that refers to an incorrect parent segment (parent id is off-by-one).

Below are the steps to reproduce the issue:

Code: Select all

# Download a model from ModelDB: https://github.com/ModelDBRepository/116094/archive/master.zip
# Extract, go to "LongDendrite" folder, and `nrnivmodl`
# Copy the lines below to a new file 'bug.py'
# Then launch NEURON with: `nrngui -python bug.py`

from neuron import h, gui

# Load a multi-compartment cell template
h.load_file('mitral.tem')

# Create an instance of the cell
cell1 = h.Mit(100)

h.define_shape()

# Manually: Tools > Model View > File > Export to NeuroML > Level 1 (or 2) > 'test.nml'
h.load_file('mview.hoc')
modelView = h.ModelView(0)
modelXml = h.ModelViewXML(modelView)
modelXml.xportLevel1('test.nml')

quit()
Then in the resulting test.nml file, the bug is visible in the first few lines (last line below):

Code: Select all

    <segments  xmlns="http://morphml.org/morphml/schema">
      <segment id="1" name = "Seg0_soma" cable = "0">
        <proximal x="0" y="0" z="0" diameter="16.2338"/>
        <distal x="50" y="0" z="0" diameter="16.2338"/>
      </segment>
      <segment id="2" name = "Seg1_soma" parent="1" cable = "0">
        <distal x="100" y="0" z="0" diameter="16.2338"/>
      </segment>
      <segment id="209" name = "Seg0_s2p" parent="0" cable = "104">  <!-- Bug: parent="0" should be "1"-->
This also happens when there is more than one cell defined. E.g. the second cell's first segment that has the second cell's root segment as the parent, ends up referring to the last segment of the *first* cell.

This happens under Ubuntu 16.04 LTS, NEURON -- VERSION 7.5 master (6b4c19f) 2017-09-25

ted
Site Admin
Posts: 5684
Joined: Wed May 18, 2005 4:50 pm
Location: Yale University School of Medicine
Contact:

Re: Possible off-by-one bug in ModelView > Export to NeuroML

Post by ted » Thu Mar 21, 2019 12:18 pm

<< Note: the original version of this post was incorrect. The create statement that needs to be commented out is the one in proc init(). My lesson, for the nth time where n is a large number, is to proofread my posts more carefully. >>

Thanks for the tip. The cause of the problem is that the template itself contains a user-generated bug. Specifically, it has two create statements, one of which is executed outside of any proc or func (it's the second line inside the template). This statement is OK. The other create statement is executed inside proc init() and is a mistake written by the person who developed this template. Both statements "create" sections that have the same names. Consequently, if you execute
objref cell
cell = new Mit()
or the equivalent Python
cell = h.Mit()
you actually get two different objects--one called Mit, which is generated by the incorrect create statement, and another called Mit[0]. Mit[0] is produced by the create statement that is not in proc init(), and its topology and anatomical and biophysical properties are as specified by the statements in proc init(). Mit, however, is just a bag of sections that have the default properties that any section has when it is first created (membrane specific capacitance, cytoplasmic resistivity, and size appropriate for squid axon, but no ion channels), and none of these sections are connected to each other. This is bad because it could easily result in a lot of confusion and incorrect results (e.g. a user could easily try assigning properties to, or recording values from, a section that actually belongs to Mit, not Mit[0]).

If you intend to reuse that code, you should comment out the superfluous create statement in mitral.tem, i.e. change the

Code: Select all

create soma, glom, prim, dend[100], s2d, s2p, p2g
inside proc init() to

Code: Select all

//  create soma, glom, prim, dend[100], s2d, s2p, p2g
All of the templates used by ModelDB entry 116094 have the same problem and should be fixed. I wonder how many other user-written templates in ModelDB also have this problem.

NEURON itself should probably be revised so that the template parsing code detects and rejects templates that have this problem.

Does NEURON's NeuroML export feature really have a bug when there is more than one instance of a cell class defined by a well-formed template? A very simple test I tried with a couple of instances of a ball and stick class didn't reveal any error, but I invite others to see what they discover.

What lessons should NEURON users draw from this?

1. Always test your code. Had the original author(s) of this model tested their code, they would have discovered their error and fixed it.
2. Always test code that you get from others. Your default assumption should be that it has bugs until you have verified that it really is what it is supposed to be. In this case, a simple count of the total number of sections would have provided a big clue that something was wrong. Simply using the ModelView tool's GUI would have immediately revealed that
cell = h.Mit()
actually produced two cells.
3. To those who would write their own hoc templates a strong suggestion: less than 5 minutes using the CellBuilder to generate a template for a toy model cell would have produced a hoc file that contains all the structure and other hints you need to do the job correctly.

JustasB
Posts: 28
Joined: Tue Dec 08, 2015 8:17 pm
Location: Tempe, AZ, USA
Contact:

Re: Possible off-by-one bug in ModelView > Export to NeuroML

Post by JustasB » Thu Mar 21, 2019 4:10 pm

Ah, that's an interesting case. I did notice the two pieces in Shape plot, but I wasn't familiar enough with the model to tell if the oddity was intentional or not.

I looked for the "multi-create" issue in the source code of the models that I have locally and was able to find a few other models where the same issue seems to appear. Take a look below (some of these just reuse an earlier model with the issue):

https://senselab.med.yale.edu/modeldb/s ... tem#tabs-2
https://senselab.med.yale.edu/modeldb/s ... tem#tabs-2

https://senselab.med.yale.edu/ModelDB/s ... hoc#tabs-2

https://senselab.med.yale.edu/ModelDB/s ... hoc#tabs-2

https://senselab.med.yale.edu/ModelDB/s ... hoc#tabs-2
https://senselab.med.yale.edu/ModelDB/S ... hoc#tabs-2

I will check on what happens with the segment ids in the multiple instantiation case.

JustasB
Posts: 28
Joined: Tue Dec 08, 2015 8:17 pm
Location: Tempe, AZ, USA
Contact:

Re: Possible off-by-one bug in ModelView > Export to NeuroML

Post by JustasB » Thu Mar 21, 2019 5:53 pm

I get the same "parent id off-by-one" issue when instantiating multiple instances of a cell made using CellBuilder.

Here is the HOC template (soma + a spiraling dendrite):
https://pastebin.com/J64KfDmZ

Then I instantiate it twice with:

Code: Select all

h.load_file('TestCell.hoc')
c1 = h.TestCell()
c2 = h.TestCell()
Then export to NeuroML with: Tools > Model View > File > Export to NeuroML > Level 1

Take a look at lines 26 and 126 in resulting NML file: https://pastebin.com/hUNaqb1e

Parent in line 26 should be "1" instead of "0" and on line 126 it should be "14" instead of "13". The other sections appear to have correct parent ids.


Interestingly, if I instantiate just one cell, the resulting NML is consistent, but the segment ids start with 0 instead of 1 when there are multiple cells. Here's the single cell NML, note lines 21 and 26: https://pastebin.com/pR1ZLMyY

ted
Site Admin
Posts: 5684
Joined: Wed May 18, 2005 4:50 pm
Location: Yale University School of Medicine
Contact:

Re: Possible off-by-one bug in ModelView > Export to NeuroML

Post by ted » Thu Mar 21, 2019 9:30 pm

Very interesting. Thanks for performing the additional tests.

By the way, my earlier post today contained an error--the erroneous create statement was the one inside proc init(). I have now fixed that.

JustasB
Posts: 28
Joined: Tue Dec 08, 2015 8:17 pm
Location: Tempe, AZ, USA
Contact:

Re: Possible off-by-one bug in ModelView > Export to NeuroML

Post by JustasB » Thu Mar 21, 2019 9:35 pm

I see the update. Thank you!

JustasB
Posts: 28
Joined: Tue Dec 08, 2015 8:17 pm
Location: Tempe, AZ, USA
Contact:

Re: Possible off-by-one bug in ModelView > Export to NeuroML

Post by JustasB » Thu Mar 21, 2019 11:36 pm

I noticed another, very similar parent-id-off-by-one issue with another model but now in a single cell instantiation scenario. The underlying cause might be the same.

Steps to reproduce:

Use this HOC file: https://senselab.med.yale.edu/ModelDB/s ... hoc#tabs-2

Note the last statement in topol() procedure:

Code: Select all

proc topol() {local i
	connect priden(0), somagc(1)
	connect priden2[0](0), priden(.8)
	connect dend[0](0), dend[1](1)
	connect dend[1](0), priden2[0](0.5) //<--- Note
}
Instantiate it with:

Code: Select all

from neuron import h,gui
h.load_file('gc-plast.hoc')
gc = h.GC()
Then ModelView > Export > NeuroML 1

This results in the following NML: https://pastebin.com/Y2qwKq1g

The issue is that on line 45, parent is "8", but the true parent id (as defined in HOC and in the NML comment) is "9".

Code: Select all

<segment id="5" name = "Seg0_dend_1" parent="8" cable = "2">  <!-- Connected at 0.5 on parent section: GC[0].priden2[0]  -->
"8" refers to "priden", but in HOC the parent is priden2[0], which has the id="9" (see below).

Code: Select all

      <segment id="8" name = "Seg1_priden" parent="7" cable = "3">
        <distal x="158" y="0" z="0" diameter="0.5"/>
      </segment>
      <!-- Section: GC[0].priden2[0] which has 3 3D points, so 2 segment(s)-->
      <segment id="9" name = "Seg0_priden2_0" parent="7" cable = "4">  <!-- Connected at 0.8 on parent section: GC[0].priden  -->
        <proximal x="128" y="0" z="0" diameter="0.4"/>
        <distal x="128" y="50" z="0" diameter="0.4"/>
      </segment>

ted
Site Admin
Posts: 5684
Joined: Wed May 18, 2005 4:50 pm
Location: Yale University School of Medicine
Contact:

Re: Possible off-by-one bug in ModelView > Export to NeuroML

Post by ted » Fri Mar 22, 2019 11:28 pm

Well, it looks like I jumped to an incorrect conclusion and must apologize for yet another mistake. There is an error in mitral.tem, but it's not what I thought was an error. The ModelView tool reports that

Code: Select all

objref cell
cell = new Mit(3) // or Mit(10) or any other number > 0
creates

Code: Select all

* 2 real cells (2 encapsulated in 1 distinct objects of 1 classes)
  * 2 Mit
    * Mit[0] root Mit[0].soma
    * Mit
But the hoc names of the cells aren't Mit[0] and Mit. hoc's topology() command generates this output

Code: Select all

|-|       Mit[0].soma(0-1)
   `|       Mit[0].dend[0](0-1)
     `|       Mit[0].dend[1](0-1)
       `|       Mit[0].dend[2](0-1)
 `|       Mit[0].s2p(0-1)
   `|       Mit[0].prim(0-1)
     `|       Mit[0].p2g(0-1)
       `|       Mit[0].glom(0-1)
|-|       Mit[0].s2d(0-1)
which reveals that there are two root sections--Mit[0].soma and Mit[0].s2d--so there are indeed two cells. mitral.tem contains no statements that would assign properties to s2d or connecti it to any other section. Most likely this is an accident, but determining the authors' intent, and whether this has serious implications for their simulation results, will require reading the original paper. The other templates in this model entry also need to be checked for orphan sections. In any case, I seriously doubt that this has anything to do with the question that you originally asked.

Now, not exactly covered with glory, I need to undo the mischief that some of my earlier remarks may have caused. First, using ModelView to examine models of individual cells created from CellBuilder-created templates (hence templates that have no user-created oddities) shows that it is normal for ModelView to report something that looks like this

Code: Select all

* 1 real cells (1 encapsulated in 1 distinct objects of 1 classes)
  * 1 Foo
    * Foo[0] root Foo[0].soma
    * Foo
even though there is only one instance of the Foo class--that is, the presence of that third line

Code: Select all

    * Foo
doesn't mean that there is a cell called Foo[0] and another cell called Foo.

Second, the mitral.tem template contains two create statements because Mit needs a numerical parameter to tell it how many sections it should create with the root name dend. In the first create statement, which is outside of any proc or func, note that dend[100] tells hoc that there may be multiple sections whose names begin with "dend" followed by an index that specifies exactly which one you refer to. The second create statement is in the template's proc init(), and is identical to the first one except that dend is followed by [$1], like this:

Code: Select all

    create soma, glom, prim, dend[$1], s2d, s2p, p2g
This signifies that the Mit class's template expects you to provide a numeric argument when you create a new instance of Mit, and the value of that argument is used to specify how many sections there will be that are called dend[j]. For example,
objref cell
cell = new Mit(10)
means that cell will actually have 10 sections with the root name dend (not the 100 that was specified in the first create statement). The fully qualilfied hoc names of these sections will be of the form
cell.dend[j]
where j lies in the range 0..9. Of course, the equivalent Python statement
cell = h.Mit(10)
would produce a Python object called cell whose 10 sections have names of the form cell.dend[j] where j lies in the range 0..9.

I'll try to clean up some of the wreckage in my previous posts over the next day or so.

JustasB
Posts: 28
Joined: Tue Dec 08, 2015 8:17 pm
Location: Tempe, AZ, USA
Contact:

Re: Possible off-by-one bug in ModelView > Export to NeuroML

Post by JustasB » Sat Mar 23, 2019 2:31 pm

Interesting. So it looks like the "orphan root" s2d section was what caused the original problem to appear.

The s2d might be safe to remove -- it's only referenced in the create statements, and I found no references to it elsewhere in the project. As you mentioned, it's not connected to anything else -- so besides additional computations, removing it should not change the model results. Although, it might change the interpretation of the results if the authors assumed the section was having an effect when indeed it didn't.

As a test, when I remove 's2d' from the two create statements, ModelView shows just one cell, and when I export to NML, the parent id is correct.

However, the problem with NML parent ids still seems to present in the case of multiple instances of a Cell Builder cell.

And a similar parent id issue appears in the case of a single instantiation of the other model, the topology() of which appears to be ok:

Code: Select all

|-|       GC[0].somagc(0-1)
   `---------|       GC[0].priden(0-1)
            `---------|       GC[0].priden2[0](0-1)
                  `|       GC[0].dend[1](0-1)
                    `|       GC[0].dend[0](0-1)

ted
Site Admin
Posts: 5684
Joined: Wed May 18, 2005 4:50 pm
Location: Yale University School of Medicine
Contact:

Re: Possible off-by-one bug in ModelView > Export to NeuroML

Post by ted » Sun Mar 24, 2019 10:43 pm

The following has nothing to do with NeuroML, but it's important anyway.

topology()'s output prompted me to look more closely at the hoc code
proc topol() {local i
connect priden(0), somagc(1)
connect priden2[0](0), priden(.8)
connect dend[0](0), dend[1](1)
connect dend[1](0), priden2[0](0.5) //<--- Note
}
which contains two booby traps just waiting to be triggered. It is inadvisable to try to connect a child section to an internal location on its parent. Why? The child section will be attached to the internal node of the parent that is closest to the user-specified location.

connect dend[1](0), priden2[0](0.5)
will attach dend[1]'s 0 end to the 0.5 location on priden2 only if priden2's nseg is odd BEFORE the connect statement is executed. If nseg is even, dend[1] will be attached to a location < 0.5 or > 0.5, depending on roundoff error. Of course, this outcome can be prevented by making sure that nseg is always odd (which is a good idea in general).

Irredeemably bad is the fact that there is NO value of nseg that results in an internal node at 0.8. Consequently,
connect priden2[0](0), priden(.8)
simply CANNOT attach priden2[0]'s 0 node to the 0.8 location on priden--instead, it will be attached to the nearest internal node, which depends entirely on priden's nseg.

"Suppose I want to attach an oblique dendrite to a precise location along a pyramidal neuron's apical dendrite?"

You have to break the apical dendrite into two sections with appropriate anatomical lengths, and attach the 0 ends of the sections that represent the oblique dendrite and the distal part of the apical dendrite to the 1 end of the section that represents the proximal part of the apical dendrite.

Post Reply