When creating a morphio.mut.Morphology, soma.type is set to undefined and is unchangable.

[wizmer]

When opening a morphology from a file, there is an heuristic that determines the soma type based on the number of points. https://github.com/BlueBrain/MorphIO/blob/master/src/morphology.cpp#L182

Unlike most of the properties of the morphio.mut.Morphology object, the soma.type is readonly as it is controlled by the heuristic described above.

The problem is that when people are creating a new morphio.mut.Morphology from scratch, the heuristic can not be applied (as the neuron is created empty) and so the soma.type is set to undefined. That can be a problem for high-level softwares that want to use the newly created morphology without having to pass by the intermediate step of writing it to disk.

One proposal (feel free to discuss it), would be that when soma.type() (https://github.com/BlueBrain/MorphIO/blob/master/include/morphio/mut/soma.h#L77) is queried and it is undefined the heuristic is called, the soma type is updated and then returned.

Since the heuristic depend on the number of soma points, maybe the soma type should also be updated every time the points array is modified.

[lidakanari]

I think it is reasonable to update the soma type if the points array is modified. Imagine that someone would like to correct the soma from one point to a contour. It is reasonable in this case to check the new type and update it accordingly.

[tristan0x]

One proposal (feel free to discuss it), would be that when soma.type() (https://github.com/BlueBrain/MorphIO/blob/master/include/morphio/mut/soma.h#L77) is queried and it is undefined the heuristic is called, the soma type is updated and then returned.

This Soma::_somaType member variable is a pain because it must be updated whenever the number of points change. What about removing it and systematically call the heuristic you mentioned above in Soma::type() member function?

Besides I would use a switch statement in function getSomaType(long unsigned int nSomaPoints) and make it noexcept instead of try/catch-ing the std::map (which is creating at every call).

[wizmer]

Yep, I think I agree with @tristan0x that we could simply rely on the heuristic all the time and get ride of Soma::_somaType

[eleftherioszisis]

+1 for the on the fly type deduction

[ohm314]

I gave this a try, seemed straight forward, but while running the tests I saw that one of the tests starts throwing a "not implemented" exception, since now the soma_three_points_cylinder.swc returns SOMA_SIMPLE_CONTOUR for it's soma type (according to the heuristic) rather than SOMA_NEUROMORPHO_THREE_POINT_CYLINDERS I guess...

======================================================================
ERROR: test_9_surfaces.test_contour_surface
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/awile/Library/Python/3.7/lib/python/site-packages/nose/case.py", line 198, in runTest
    self.test(*self.arg)
  File "/Users/awile/projects/circuit/morphio/tests/test_9_surfaces.py", line 46, in test_contour_surface
    assert_almost_equal(Morphology(os.path.join(_path, "soma_three_points_cylinder.swc")).soma.surface,
morphio.MorphioError: Surface is not implemented for SOMA_SIMPLE_CONTOUR

So, I'm not sure getting rid of _somaType completely was the best thing? (also, I've spent just a couple of hours with this code, so sorry, if this was a rather naive question...).

[cattabiani]

Ok, Ioannis and I digged out a bit the problem. We discovered that the "heuristic" function that you were quoting @tristan0x (getSomaType) is called everywhere except for the reader morphologySWC:

if (version() != MORPHOLOGY_VERSION_SWC_1) _properties->_cellLevel._somaType = getSomaType(soma().points().size());

That class, that is declared and implemented in morphologySWC.cpp (no header), calls its own somaType(). It differs from getSomaType() and is not a static function (it uses various internal variables).

From Soma or Mut::Soma we cannot see morphologySWC::somaType() because (among other reasons) it is in a cpp file. We could (maybe) try to incorporate the behavior of somaType() in getSomaType(). However, it looks like this convoluted ad-hoc function was done in this way on purpose. I would like to have confirmation from someone that has more experience on this code than me.

Shall I proceed?

[ohm314]

well since I have already written a bit of code on this maybe let me look a bit further into it. Sorry, didn’t know you guys were looking at it since it was unassigned :/

[ohm314]

I had some more time to look into this. While getting rid of the _somaType member, I also changed the constructors of the Soma class:

https://github.com/BlueBrain/MorphIO/blob/master/src/mut/soma.cpp#L15-L21

In particular, if we don't store anymore the _somaType, then we cannot copy at construction the some type from the mutable soma (line 20). We could in this case set an internal flag that tells type() to get the soma type through _properties->_cellLevel._somaType rather than the heuristic. @wizmer @lidakanari @eleftherioszisis what do you think?

[wizmer]

Good point @ohm314. So let's summarize:

• We want an heuristic to find out the soma type of a mutable morphology
• Unfortunately, because the same points are understood differently by different file format, we have an indirection on the heuristic. In short, we cannot determine the soma type without knowing the output file format.
• If a mutable morphology is constructed from an immutable one, we want to use the soma type of the latter. However, if the user mutates the mutable soma later on (ie. by changing the points), we want to use the heuristic.

Welcome to the wonderful world of morphologies ! I'm tempted to remove the "good for beginner flag" now, ahah.

Proposal: Step 1: @ohm314's proposal: if the soma is created for an immutable soma, use an internal flag to use _properties->_cellLevel._somaType. The flag must be discarded if the soma points array is mutated.

Step 2: Use the heuristic, otherwise. The heuristic for less than 3 points soma is clear. The problem arises for 3 or more points. You could use the following rule:

Step 2.1: 3 point soma: If the 3 points follow the SWC specification http://neuromorpho.org/SomaFormat.html assume it is a soma_neuromorpho_three_point_cylinders The 3 points should exactly match the following spec:

points	X	Y	Z	R
1st point:	x	y	z	r
2nd point:	x	(y - r)	z	r
3rd point:	x	(y + r)	z	r

If the 3 points don't match this, assume it is a soma contour

Step 2.2: The soma has more than 3 points: Assume it is soma contour. It could eventually also a stack cylinders soma but that's unlikely and this soma format makes little sense anyway.

Finally, I would understand if you guys don't have much time to spend on this so it's up to you to work on it or not. It's not an urgent issue anyway so it can wait until next free day :) Or I can take over from here if needed. Please let me know.

[ohm314]

Thank you for taking the time and the detailed instructions @wizmer ! Nah, I'll continue looking into it :) Good chance to learn a little about morphIO.

[ohm314]

If you're willing to indulge my questions some more, @wizmer ...

Spent a little more time with this. My approach so far is to take checkNeuroMorphoSoma out of the SWC reader class and make it usable from both the mutable soma and the reader to do the necessary check. However, what I noticed is that the Sample type offers a "diamaterfield, which is lost in themorphio::Point` so then it is not clear to me how to use the same routine for both cases.

Also, @wizmer, in your explanation you refer to r, is it then shared between all points once one has a valid soma, while Sample cannot assume that yet? Should the new checkNeuroMorphoSoma function take for each point a diameter but when it is called from Soma, take all diameters as the soma diameter?

[wizmer]

Hi @ohm314 Sorry for the delay. Feel free to change the signature of checkNeuroMorphoSoma to match both implementation (maybe passing the points and the diameters as two separates arguments). Or maybe by using an intermediate std::array<float, 4> structure in both cases ?

No, the radius is not shared between the points. In all cases, each point should have its own diameter. After all, you can't know what is the user about to do with the soma. Maybe he'll open a valid 3 point soma file and modify it to something which is no longer a 3 point soma. So it needs to be able to modify each radius independently.

Does this answer you questions ? Feel free to contact me directly on Slack otherwise.

[mgeplf]

In order to address soma writing, I made the mut soma type modifiable/changeable: https://github.com/BlueBrain/MorphIO/pull/458/files#diff-55cc46e5a8b6cee527ea51dde896a37e429922a3ce40d3ff824308e1d4eacf0dR40

Other than when loading morphologies, I think we should keep the autodetection to a minimum, and leave the choice up to whoever is modifying the morphology.

Since this is an older ticket, I will close it.