-
Notifications
You must be signed in to change notification settings - Fork 45
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
Set pos 3d fixed #4
Conversation
@alejoe91 I cannot reproduce the problem you're seeing. I set up these test files:
and
With the current master branch the soma positions are correct ([ 0. 0. 0.], [ 0. 0. 0.], [ 10. 0. 10.], [ 30. 0. 30.]) for the different x,z-offsets, as well as pt3d and segment coordinate info: Notice on the first iteration the However on your pull request the cell's soma's center position is no longer in (0,0,0) and pt3d info and segment coordinate info do not overlap as they should: |
Hi, I was wrong in deleting the if-else, because the start, end, and midpoints are updated in _update_pt3d (which calls _collect_geometry()). So I was basically moving them twice. I think that the other fix (passing diffx, diffy and diffz to _set_pt3d_pos()) is correct, though. In fact, if you set_pos(50,0,0) for three times in a row:
so if the soma position hasn't changed, the 3dpoints are moved anyways. Do you agree? I just committed a corrected version (commit 'reinserted if-else, kept diffx, diffy, diffz') |
@alejoe91 I was mistaken above: the last position was (30,0,30) on the master branch when it should have been (20,0,20). Sorry about that. However, I reran now with your code, and the cell position is not correctly instantiated to (0, 0, 0) when pt3d==True. For the simplified geometry above the result is (0,0,-15), however if pt3d==False the result is (0,0,0). The issue with non-overlapping geometry appears fixed though. |
@espenhgn Anyway, try the new version (after the first time the soma and the segments should always in the correct position) and let me know :) |
There is a call to |
I saw that, that's why I was quite surprised to see that the first time it doesn't set the position correctly. |
For these morphology files the root of the morphology (the 1st data point for the first section, typ. the soma) may be some arbitrary location in 3D space. For consistency between different cell models in LFPy we made a decision that the "default" cell soma position should be having it's mid point at (0,0,0), not one of it's end points. That should also be the case if the soma.nseg > 1. |
Ok, then I will try to dig deeper and see where it gets displaced. |
@espenhgn
What do you think about it? |
I think that should work for now. Thanks for your help! |
Added celsius parameter to cell class, and updated example2.py
Cell.set_pos() fixed: