Back to the main page.
Bug 790 - UI improvements to rejectvisual_summary
| Status | CLOSED FIXED |
| Reported | 2011-06-29 23:54:00 +0200 |
| Modified | 2011-07-29 10:34:01 +0200 |
| Product: | FieldTrip |
| Component: | preproc |
| Version: | unspecified |
| Hardware: | PC |
| Operating System: | Mac OS |
| Importance: | P1 enhancement |
| Assigned to: | Jörn M. Horschig |
| URL: | |
| Tags: | |
| Depends on: | |
| Blocks: | |
| See also: |
Eliezer Kanal - 2011-06-29 23:54:12 +0200
Created attachment 85 UI update for rejectvisual_summary I've overhauled the rejectvisual_summary interface, making the entire thing work through uicontrols. It now functions similarly to the other rejectvisual scripts. While it looks like I've changed virtually everything, most of the changes are due to how guidata works; for example, I had to change all instances of `ntrl` to `info.ntrl`. Take a look at the code and let me know what you think. As far as I can tell, it's working without problems. There is a known bug with the "show rejected?" button as follows: 1) reject a channel 2) turn on "show rejected" 3) switch metrics At this point, the rejected channel will not show up. Simply reject it again (i.e., reject twice) and it will show up as expected. I've also beefed up the input handling a little bit (it won't choke if brackets aren't supplied around multiple numbers), but it still could use some more work, as there's no catch if the user supplies too large of a number for either trials or channels. Please let me know what you think.
Eliezer Kanal - 2011-06-30 00:01:55 +0200
Comment on attachment 85 UI update for rejectvisual_summary Changing attachment to a patch. One more comment: the whitespace is all messed up, because whenever possible I didn't change it from the previous version, in the hopes that it would make the diff easier to read. I think that's kinda unnecessary now, what with so much being changed, but *shrug*. I can fix that after the fact, if necessary.
Eliezer Kanal - 2011-06-30 16:19:38 +0200
Created attachment 86 UI update for rejectvisual_summary bugfix for toggle_rejected function
Jörn M. Horschig - 2011-07-01 09:00:57 +0200
I'm gonna check whether it works for me - I just started using that function for my data anyway
Jörn M. Horschig - 2011-07-01 09:56:27 +0200
Hey, okay, I checked it out, here is my list of complaints ;) I think the function definitely benefits from having a UI control. However, a definite must-have is the mouse-selection of trials and channels (because that's how people here use the function). Let me know what you think of the other suggestions! 1) How can I select trials? I cannot figure out how to either rejecttrials, or look at trials (same for channels). The only way I see is clicking on individual trials, check it's x-value and insert the value in the corresponding field (e.g. 'toggle trial'). Could you maybe add a button so that you can select trials using the mouse (like it was before)? The next point is related to this. 2) It may be a personal taste, but I would rather go for buttons and complete mouse-control, rather than inputfields. e.g., add a button for 'reject'. If the user then selects one or more trials (or channels), they get rejected. This way, you also would not need to separate toggle trial and toggle channel (I would expect that there is a way to find out in what subplot the user has clicked?). Similarly, to toggle back, you could add an 'accept' button, which would mark all artifacts within a certain range as accepted. 3) Selecting a radiobutton starts calculating a metric, but if you maximize the window, the user does not know what is happening (because the info for this is given as text output). Given that there is now a UI control, I expected that all relevant information is shown in the GUI. This led me to wildly clicking on various radiobuttons :) This is also linked to the fact that if you click on one radiobutton, the old selection stays active until the metric computation is finished. I would suggest to deactivate the radiobuttons while the metric is being computed. 4) I like the 'show rejected' button ;) Maybe plot rejected trials in red? Just a minor thing though. Of course, we should discuss these points, as they might reflect my personal preference, but I think they might help to improve the UI. Greetings, Jörn
Eliezer Kanal - 2011-07-01 21:21:49 +0200
(In reply to comment #4) > okay, I checked it out, here is my list of complaints ;) > I think the function definitely benefits from having a UI control. However, a > definite must-have is the mouse-selection of trials and channels (because > that's how people here use the function). Let me know what you think of the > other suggestions! So, mouse selection is still available, the same way it was before: enter "0" into the appropriate textbox and draw a box using the mouse. The fact that it isn't actually indicated anywhere is just because I forgot to make that clear. I want to keep the amount of static text to a minimum, though... I think we could add a "help" button that brings up a window explaining how to use the interface. Although, this particular bit could be moot, because of your next point. > 2) > It may be a personal taste, but I would rather go for buttons and complete > mouse-control, rather than inputfields. e.g., add a button for 'reject'. If the > user then selects one or more trials (or channels), they get rejected. This > way, you also would not need to separate toggle trial and toggle channel (I > would expect that there is a way to find out in what subplot the user has > clicked?). Similarly, to toggle back, you could add an 'accept' button, which > would mark all artifacts within a certain range as accepted. I like the idea of avoiding the need to specify which plot you want to work in. Assuming we can work around the subplot problem, that's a very good suggestion. I'll start looking into it. I'm not sure I follow you with the accept button. It sounds similar to the "view rejected" button to me. Could you clarify what you mean by "artifacts within a range as accepted"? > 3) > Selecting a radiobutton starts calculating a metric, but if you maximize the > window, the user does not know what is happening (because the info for this is > given as text output). Given that there is now a UI control, I expected that > all relevant information is shown in the GUI. This led me to wildly clicking on > various radiobuttons :) > This is also linked to the fact that if you click on one radiobutton, the old > selection stays active until the metric computation is finished. I would > suggest to deactivate the radiobuttons while the metric is being computed. Heh... that's what happens when I develop the interface using a dual-monitor setup. Very good point. I think the best way to address this would be to add a log-style non-clickable textarea at the bottom of the window to hold all that text. > 4) > I like the 'show rejected' button ;) Maybe plot rejected trials in red? Just a > minor thing though. We could make the visuals of the rejected points a user-definable cfg parameter without much difficulty. Let me know what you think, and I'll begin working on version 2 of this script.
Jörn M. Horschig - 2011-07-02 09:25:23 +0200
(In reply to comment #5) > I'm not sure I follow you with the accept button. It sounds similar to the > "view rejected" button to me. Could you clarify what you mean by "artifacts > within a range as accepted"? Oh sure, maybe I was a bit unclear :) With "artifacts within a range" I meant the procedure of using the mouse for trial selection. And, the idea I had with the accept button is basically this one: If you think about removing all textfields, you need a way to 'toggle trials'. An intuitive way would be to have two buttons, one for rejecting trials by mouse selection, and one for accepting trials (re-rejecting so to say) by mouse selection. The latter one does of course only make sense if 'view rejected' is on. You could also use the same button for this, and mouse selection is rejecting trials if 'view rejected' is off, and accepting if it is on. If you get the idea, you can maybe make something smart out of this ;) > Heh... that's what happens when I develop the interface using a dual-monitor > setup. Very good point. I think the best way to address this would be to add a > log-style non-clickable textarea at the bottom of the window to hold all that > text. Hm yeah, I was thinking about something similar for the databrowser, too. However, Robert did not want such a thing in the databrowser, although I think it would be nice. Maybe you can implement it so that he gets convinced ;) > We could make the visuals of the rejected points a user-definable cfg parameter > without much difficulty. Nice, yeah ;) > > Let me know what you think, and I'll begin working on version 2 of this script. Thanks for all the work :)
Eliezer Kanal - 2011-07-06 20:56:01 +0200
Created attachment 87 UI update for rejectvisual_summary v3 I've updated a number of things, and I tried to incorporate the comments. To summarize: 1) Instead of going for a "reject/accept" button, I just set it up so any activity on either the trials or channel plot will reject/accept trials in the box. Basically, all you need to do is drag now. I think this is the behavior we're looking for... please try it out and let me know. 1a) I've added a "Rejected channels:" (and separate one for Trials) textarea, which shows which channels/trials have been rejected. 2) I've added a terminal-style log window at the bottom. You'll note that new text is added at the top of the window (instead of at the bottom) - unless we want to start screwing around with undocumented matlab functions (see http://undocumentedmatlab.com/blog/setting-line-position-in-edit-box-uicontrol/), this is how we're gonna have to do this. 3) A bunch of layout changes. Note that there is still at least one bug with how "show rejected" works (easily reproducible; (1) run with "viewmode=reject", (2) reject a trial, (3) turn on "show rejected"; it's not shown.) I'm not sure what the problem is. Please let me know what you think.
Jörn M. Horschig - 2011-07-07 10:48:24 +0200
Hey, great, looks good and is easy to use! I like the log-info a lot, and that there is no use to push a button at all. Although, there is still one problems (and some minor): 1) I found a new bug, but it worked the second time I started the function, so let's blame matlab - the prob was that I could not remove the very last trial. Anyway, it worked the second time. 2) After having removed a channel and switched 'show rejected', it does not re-appear. 3) Furthermore, after I removed a channel (or trial) and played a bit around with 'show rejected', the mouse selected area does not coincide with the channels being removed. I think there is some error in the horizontal-offset. This happened to me also when just having removed one trial, without having touched any other part of the UI. 3) If having removed an outlier and switch 'show rejected' on, the vertical limits do not change, therefore not showing the outlier anymore 4) Maybe reduce font-size for the caption/labels - not sure what the resolution of most people is, when I just reduced the window size it began to look a bit ugly. 5) Is it possible to reject using the left mouse button and 'accept/re-reject' using the right mouse button? You could then add to the blue text something like 'left button: reject trials/channels, right button: accept trials/channels'. I think it would be nice. Btw, just googled how to do it: http://www.mathworks.com/matlabcentral/answers/9222-mouse-button-click 6) Not sure if this is a real problem, I removed all trials but one. If removing the last one, others will show up again - I think you do not have to bother with this, no one will ever do this ;) Could you try looking especially into the third one? This is quite strange
Jörn M. Horschig - 2011-07-07 10:52:49 +0200
just found that I got the very first problem I stated occured again, after having removed a channel - not quite sure what is happening there.
Eliezer Kanal - 2011-07-07 15:58:18 +0200
I'll re-examine the "show rejected" problems. The problem is that the code was originally not intended for the "show rejected" use, and this will probably involve a rewrite of some parts of the original code, which I was hoping to avoid, but I think it'll be necessary. I'm busy until next Tues, but I'll work on it then. > 4) Maybe reduce font-size for the caption/labels - not sure what the resolution > of most people is, when I just reduced the window size it began to look a bit > ugly. I'll play around with that. It'll > 5) Is it possible to reject using the left mouse button and 'accept/re-reject' > using the right mouse button? You could then add to the blue text something > like 'left button: reject trials/channels, right button: accept > trials/channels'. I think it would be nice. Btw, just googled how to do it: > http://www.mathworks.com/matlabcentral/answers/9222-mouse-button-click That's a very good idea, I'll try to implement that.
Eliezer Kanal - 2011-07-07 16:02:48 +0200
Just realized my second comment wasn't complete; I meant to write that it will be somewhat dependent on taste, but I'll try to make it look good.
Jörn M. Horschig - 2011-07-07 16:36:03 +0200
Hm, I see the point. I guess it turns out that the UI update is more work than expected :) Anyway, I appreciate that you are working on this, and I really think that the function has already improved in terms of usability. Anyway, I'll better wait with committing the change until the update is finished. Take your time, of course there are other things which are more important :)
Eliezer Kanal - 2011-07-14 20:45:43 +0200
So, I've realized that the "toggle" feature, which I'm a big fan of, has a pretty fundamental problem in it's current realization. The idea is that channels/trials which aren't selected will still show up, so that the user can easily turn them back on. The problem is that, if there are both channels and trials deselected, there's no way to intelligently place the dots. In more detail, consider the fact that deactivating a trial (for example) changes the values in the heatmap plot, and changes the maximum values of the channels. If I deactivate a few bad trials, my channel maximum values have now changed. Now consider that, after I've deactivated a bad trial, I deactivate a channel. I now have changed the values of all my trials, as well. At this point, there's no way to accurately predict what the value should be for *deactivated* trials, since the channel layout was dependent on the selection of trials. It's a catch-22... I can't set trial values without knowing channel values, and I can't update the channel values without knowing trial values. As such, if a trial wasn't used to calculate channel values, subsequent changes to the channel values will yield no meaningful value for deactivated channels. This is a problem because we use trial/channel values to do the visual selection. Without a value, points aren't able to be selected using visual selection. The only thing I can think of is to put all deactivated points at a particular place - say, the bottom of the plot - and allow them to be selected there, but that kind of defeats the purpose. Any thoughts?
Jörn M. Horschig - 2011-07-15 09:17:04 +0200
I am not quite sure if I understand it correctly, but does this problem occur only if the user can decide to switch either channels or trials back, and the problem would be solved if the user can only toggle both at the same time? Just in case this solves the issues, you could store two colormaps and two axes when calculating the metric, one for all trials/channels and one for only selected trials/channels, so that you won't have to recompute the metric when toggling. Not sure if this is the core of the problem though ;)
Eliezer Kanal - 2011-07-15 16:16:47 +0200
(In reply to comment #14) > I am not quite sure if I understand it correctly, but does this problem occur > only if the user can decide to switch either channels or trials back, and the > problem would be solved if the user can only toggle both at the same time? Just > in case this solves the issues, you could store two colormaps and two axes when > calculating the metric, one for all trials/channels and one for only selected > trials/channels, so that you won't have to recompute the metric when toggling. > > Not sure if this is the core of the problem though ;) Sorry for the confusion; I'm finding it tough to explain. This is actually the opposite of what you said; it's a problem because the user is able to change both trials and channels, which is kinda the whole point of the summary function. When using the "toggled" view, if both channels and trials have been toggled, there's no good way way to plot the "unselected" datapoints (i.e., datapoints set to 0 in chansel/trlsel). In the interest of getting something done, I think I'll just remove the toggle functionality until I can think of a better way to address it, if one exists. Ah well, it was good while it lasted :)
Eliezer Kanal - 2011-07-15 21:59:59 +0200
Created attachment 97 UI update for rejectvisual_summary - working code OK, this version seems to work. I've removed the "toggle" functionality and made it easier to use the textboxes. I also added functionality so that you don't have to type the index number for a given channel, but you can type the label. For example, previously if I wanted to reject channel MEG1133, I would need to type "121" (the index number). I've enabled it to take either text or label, and it will intelligently figure out what you're trying to do. I looked through the various label files to try to ensure it will work for different types of datafiles, but that part definitely needs to be tested on something other than Neuromag data. I didn't change the font sizes much, but I moved a few things around; let me know if it still looks ugly.
Jörn M. Horschig - 2011-07-19 10:38:34 +0200
Hi Elli, ah, I see the problem with toggling... hm, the only way I would see is to have a separate toggle channels and toggle trials button. Would be great, but definitely no must-have. Apart from that, it looks likes your done :) I like especially the toggling channels, i.e. that you can use channel labels or numbers, and that, after rejecting a channel, you see it's label. I removed the # from the label, so that is is more obvious for users that they may use channel labels. I hope that is okay with you! Some minor things, that I quickly fixed: I fixed a bug in the z-value metric - the problem was that you intialize runsum=zeros(info.nchan, info.ntrl), but since you add sum(dat, 2), it needs to be nchanx1. I changed that (line 141). Also, you initialized runss with only the number of selected channel, but dat has the size of all channels. I changed that as well. And, the label for rejected trials said channels instead (line 250). The layout looks a bit 'unclean' (e.g. the white background of the metric buttons extends into the instruction label), but I do not consider this as being an issue :) Since it all works fine for me, I committed your patch and marked this bug as resolved! Thanks for your work! I find the function more easier to use now, and will also use it more often now.
Eliezer Kanal - 2011-07-19 15:26:58 +0200
Created attachment 102 screenshot of rejectvisual_summary UI on launch Thanks for taking care of cleaning up the code. You know better than I how easy it is to screw up a 1xN matrix with MxN or something. Thanks! (In reply to comment #17) > The layout looks a bit 'unclean' (e.g. the white background of the metric > buttons extends into the instruction label), but I do not consider this as > being an issue :) That's odd, I don't see that on my machine. I'm attaching a screenshot of what it looks like on my machine, can you attach a screenshot of the UI on yours?
Jörn M. Horschig - 2011-07-20 11:03:44 +0200
Created attachment 103 UI on windows OS