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

ENH: Add new electrode without linear interpolation #4

Closed
choldgraf opened this issue Nov 30, 2015 · 7 comments
Closed

ENH: Add new electrode without linear interpolation #4

choldgraf opened this issue Nov 30, 2015 · 7 comments

Comments

@choldgraf
Copy link
Contributor

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
Copy link
Owner

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
#4.

@choldgraf
Copy link
Contributor Author

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
Copy link
Owner

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
Copy link
Contributor Author

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
Copy link
Owner

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
Copy link
Contributor Author

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

@aestrivex
Copy link
Owner

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

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

No branches or pull requests

2 participants