Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

section points and lengths don't match #330

Closed
cjayb opened this issue May 11, 2021 · 6 comments · Fixed by #373
Closed

section points and lengths don't match #330

cjayb opened this issue May 11, 2021 · 6 comments · Fixed by #373
Labels
good first issue Good for newcomers

Comments

@cjayb
Copy link
Collaborator

cjayb commented May 11, 2021

For example (in params_default.py):

  • in get_L5Pyr_params_default(): 'L5Pyr_apicaltrunk_L': 102., but
  • in _secs_L5Pyr(): sec_pts = { ... 'apical_trunk': [[0, 23, 0], [0, 83, 0]], ...

That is, based on the 3D section end points, the apical trunk is 60 microns long, whereas the L-parameter is explicitly set to 102. NEURON uses L in simulations.

I propose that this be fixed by adjusting the sec_pts to match the L-values. WDYT? Could be a good First Issue for someone?

Related to #165 and #327
Ping @jasmainak @ntolley @rythorpe

@jasmainak
Copy link
Collaborator

+1, I can take care of this with a couple of other things I had in mind. @rythorpe did you have a PR upcoming / local changes or can I take a go at the code?

@rythorpe
Copy link
Contributor

I can see how this might be a problem if we're using sec_pts to plot morphology. Maybe after this is fixed we can simplify the code by removing the lines where L is set altogether, and assume moving forward that morphology will be set singularly by specifying endpoints. Either that or we add a method that updates sec_pts whenever length is set (to accommodate future models being read in that rely on length specifications).

@jasmainak I don't have any local changes yet but was planning on diving into it tonight. If you're cool with waiting, that'd be ideal.

@jasmainak
Copy link
Collaborator

Okay, go for it! I have other things on my plate for now :)

@jasmainak jasmainak added the good first issue Good for newcomers label May 20, 2021
@jasmainak
Copy link
Collaborator

jasmainak commented May 20, 2021

Note that Neuron adjusts L and section points so that they match. So, I'm assuming whatever was set later is used for the simulation. Looking at our code, it seems that it is L (which can be adjusted from the param file)

@cjayb
Copy link
Collaborator Author

cjayb commented May 20, 2021

Yikes, that's pretty bold behaviour! We should throw mismatch exceptions rather than counting on Neuron. Isn't this related to #314? In the sense that we have to make a choice on what our primary source of geometric information is. In #334 you mentioned that

what makes the most sense to me is to write up a reader for the morphology of the cells and store the geometry of the default cells in that file format. Then, finally deprecate param keys related to the geometry in the param file.

I really like the idea of storing cell morphology in a file, and infer sec.L from the geometry. We need a format that's powerful enough to represent the sec.diam, too. Though it is currently an overkill, how about going with a FAIR format? I've noticed that many of the cells published with papers in recent years are ASC-dumps of the Neurolucida file format by MBF Bioscience, who in addition to the above bioRxiv paper have written a detailed specification. Supporting this reader might future-proof us, as discussed on Tuesday.

@jasmainak
Copy link
Collaborator

jasmainak commented May 20, 2021

Okay let's think of the right file format then. I'll get the ball rolling here with a table, you guys can edit and make it more detailed:

---- NeuroML Neurolucida ASC SWC format
Open specification? Open but very little explanations. Good examples Y
Membrane capacitance/resistivity? Y --
Validator? Y --
Software openly available? Y Trial version available
Reader in Neuron? Y Y Y

I think we could delegate the reading to Neuron by the way, same as LFPy folks have done. Then we could get all these different readers for free. Disadvantage might be that some numerical precision might be lost? For the ASC format, I can't figure out if the membrance capacitance and resistivity can be stored. Is that going to be stored in our file as well or do we populate it later?

In any case, we need to decide what's the right file format to store the default cells in. I'm leaning towards NeuroML based on the fields I have filled so far ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants