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

Rejected components aren't handled properly-attempts to plot fail #6

Closed
rutilate opened this issue Jun 21, 2021 · 3 comments
Closed

Comments

@rutilate
Copy link

rutilate commented Jun 21, 2021

Rejected dipoles are not properly identified and prevented from plotting.

Rejecting dipoles doesn't reject the component associated with that dipole. Plotting and other functions will check for an invalid component, but do not check for an invalid dipole.

Because of this failure to check, an invalid component cannot be rejected from the component map, leaving obviously bad components in the data.

If you call dipfit_reject() (for example as is called from pop_multifit()), any dipoles above the threshold input argument are deleted by setting RV=1, and every other field= [].

However, without custom coding, the component associated with that dipole is still erroneously valid. Much of the code in dipplot, pop_viewprops, etc. will test for a valid component and check to ensure dipfit has been run, but will not test for a valid dipole before plotting.

Given this inconsistent state, choosing the invalid component from pop_selectcomps() will turn the background of the IC display window black and remove all the button controls, preventing the user from rejecting the invalid component.

At the lowest level, dipplot.m needs to check for an empty dipole before beginning to plot. At the very least, this would look like this at ~ line 230 (see file attached with .m replaced with .txt for github compliance; search for CNB in the file):

% CNB ZZZ: check to make sure not dealing with an empty dipole; ideally
% would display a message in the GUI indicating an invalid dipole
if(isempty(sources.posxyz))
warning('Warning: empty dipole found, skipping dipole plotting');
return;
end
dipfit_reject.txt

In a similar vein, dipfit_reject.m sets the RV = 1 on line 53, BUT code elsewhere is testing for and setting the value to 'N/A' (see pop_prop_extended.m line 408 where it tests for dipfit having been run but then fails to stop trying to plot the empty dipole).

I've added code in pop_prop_extended() to label the dipole as INVALID and prevent the display of any MRI. This also prevents the background from being painted black and all the control buttons at the bottom from being displayed, making it impossible to reject the component.
% CNB ZZZ: Check here to see if this dipole is empty. If
% empty, break out of the loop, no dipoles to print
if(isempty(EEG.dipfit.model(chanorcomp).posxyz))
warning('No dipole to plot');
% Display warning text
[dip_title, ~] = title(dip_background, 'Dipole Location', 'INVALID DIPOLE', 'FontWeight', 'Normal');
set(dip_title,'FontSize',14);
break;
end
File is attached (renamed to .txt); search for CNB.
pop_prop_extended.txt

I've created an issue in the ViewProps plugin that links to here.

@arnodelorme
Copy link
Contributor

arnodelorme commented Jun 21, 2021

Yes, these components are labeled for rejection not rejected. You can also remove the components and then they will not appear anymore in any plots.

@rutilate
Copy link
Author

Why wouldn't you at least want to improve your error checking to include the dipole if you're already checking for a bad component?

And how do you delete the component based on the rejected dipole?

@arnodelorme
Copy link
Contributor

To remove components, you can use "Tools > Remove Components from data"
Tx

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