search

aestrivex / ielu

grids and strips electrode localization utility
GNU General Public License v3.0
10 stars 7 forks source link

ENH: Add new electrode without linear interpolation #4

Closed choldgraf closed 8 years ago

choldgraf commented 8 years ago

I've been trying to add electrodes manually, but can't figure out how to do this. Right now, clicking "add blank electrode" seems to assume that you want to do some sort of linear interpolation. It would be much better to allow you to click on the CT manually to say where the electrodes are.

If I add a new blank electrode, then click on the "place manually" dropdown button, I get this error:

  File "/home/knight/holdgraf/anaconda/lib/python2.7/site-packages/pyface/ui/qt4/action/action_item.py", line 160, in _qt4_on_triggered
    self.controller.perform(action)
  File "/home/knight/holdgraf/anaconda/lib/python2.7/site-packages/traitsui/qt4/ui_base.py", line 138, in perform
    self.ui.do_undoable( self._perform, action )
  File "/home/knight/holdgraf/anaconda/lib/python2.7/site-packages/traitsui/ui.py", line 715, in do_undoable
    action( *args, **kw )
  File "/home/knight/holdgraf/anaconda/lib/python2.7/site-packages/traitsui/qt4/ui_base.py", line 143, in _perform
    method( self.ui.info )
  File "/home/knight/holdgraf/src/python/gselu/electrode.py", line 560, in do_manual_reposition
    x,y,z = self.cur_sel.asct()
ValueError: need more than 0 values to unpack

Perhaps a simple quick fix to this would be to specify a default x/y/z point at the center of the CT.

aestrivex commented 8 years ago

Right now the program expects you to:

a) Create a blank electrode b) Fill in its geometry (i.e this is point 3,7) c) Click the "linearly interpolate" button.

This is mentioned briefly in the documentation.

I agree it would make more sense to be done graphically.

On Mon, Nov 30, 2015 at 4:18 PM, Chris Holdgraf notifications@github.com wrote:

I've been trying to add electrodes manually, but can't figure out how to do this. Right now, clicking "add blank electrode" seems to assume that you want to do some sort of linear interpolation. It would be much better to allow you to click on the CT manually to say where the electrodes are.

If I add a new blank electrode, then click on the "place manually" dropdown button, I get this error:

File "/home/knight/holdgraf/anaconda/lib/python2.7/site-packages/pyface/ui/qt4/action/action_item.py", line 160, in _qt4_on_triggered self.controller.perform(action) File "/home/knight/holdgraf/anaconda/lib/python2.7/site-packages/traitsui/qt4/ui_base.py", line 138, in perform self.ui.do_undoable( self._perform, action ) File "/home/knight/holdgraf/anaconda/lib/python2.7/site-packages/traitsui/ui.py", line 715, in do_undoable action( _args, *_kw ) File "/home/knight/holdgraf/anaconda/lib/python2.7/site-packages/traitsui/qt4/ui_base.py", line 143, in _perform method( self.ui.info ) File "/home/knight/holdgraf/src/python/gselu/electrode.py", line 560, in do_manual_reposition x,y,z = self.cur_sel.asct()ValueError: need more than 0 values to unpack

Perhaps a simple quick fix to this would be to specify a default x/y/z point at the center of the CT.

— Reply to this email directly or view it on GitHub https://github.com/aestrivex/gselu/issues/4.

choldgraf commented 8 years ago

Yeah, I think it just comes down to how much your average user will trust linear interpolation. Most of the ecog labs I work with are highly distrusting of automatic anything, so they really want to be able to see the raw CT and pick things by hand.

What I've been doing is just picking a "noise" electrode, and then manually changing its position.

aestrivex commented 8 years ago

I think the linear interpolation solution in principle is the right one. Having the user pick them manually from the CT is also fine, but linear interpolation will work well enough that it isn't a problem.

When I was initially designing this and before I got around to building the embedded CT visualizer (which my PI told me I needed to do and I was skeptical it would be useful and he was right), I envisioned having some "automatic correction procedure" that extracted new electrode locations from the CT with a more lenient threshold using the electrodes that had already been identified and confirmed correct by the user. Which was complicated that it worked very poorly and would have taken a huge amount of work to do it correctly.

Adding the missing electrodes via linear interpolation turned out to be much smarter since the majority of the time, the electrode that's missing is one of the ones in the middle.

That left the problem that you couldn't usually figure out the ones on the edge, so I built the existing CT window to deal with that. Maybe that makes the linear interpolation part obsolete but I still think the linear interpolation algorithm is just as good as what a user can do manually most of the time, if the current UI could make it more obvious how to do it.

choldgraf commented 8 years ago

So, I totally agree with you. I think that there are two questions here: one is "what is the most optimal solution?". And the other is "what are people going to want to use?". I agree with you that linear interpolation is probably the best way to do this generally, I'm just saying that in my experience in talking with a lot of ecog researchers out there, they tend to want to do tweaking manually by hand.

For example, here are two other electrode selection tools that have been developed in matlab:

Note that both of them use CT-based electrode selection. They actually don't do any automatic extraction. This is what potential users will be used to and probably what they'll want to use to some extent, at least until you can convince them that using linear interpolation w/ an automatic extraction algorithm is better.

If you guys are just developing this tool internally, I think whatever works for you is fine. If you're hoping for a more general adoption, I think it's a question of what users might want. I understand whatever direction you take with it, just giving my 2 cents.

aestrivex commented 8 years ago

Well, the direction I prefer is to allow the user to do it in either way they want. The linear interpolation algorithm is actually smart so it should be included, but the CT window needs to be powerful enough that the user can do whatever they want in that too.

choldgraf commented 8 years ago

Good point - to be clear, I definitely am not saying "don't do linear interpolation", it seems like a really useful approach.

aestrivex commented 8 years ago

CT window not perfect, but allows this behavior as of whenever it was implemented