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

BUG: report can contain stale stats when one subject is used #900

Open
SophieHerbst opened this issue Mar 22, 2024 · 6 comments
Open

BUG: report can contain stale stats when one subject is used #900

SophieHerbst opened this issue Mar 22, 2024 · 6 comments
Labels
enhancement New feature or request

Comments

@SophieHerbst
Copy link
Collaborator

I managed to fool myself by forgetting to remove the subject variable from my config after debugging, and looking at the sub-average report, which was done for only one subject.
I think this could be prevented by printing the subject count in the report any time a figure is added, in particular for the decoding plots.
For example:
title = f"CSP TF decoding: {cond_1} vs. {cond_2}, N = {n_sbs}"
I can give it a go if you agree.

@hoechenberger
Copy link
Member

I agree 😀

@hoechenberger hoechenberger added the enhancement New feature or request label Mar 22, 2024
@larsoner
Copy link
Member

Agreed!

@SophieHerbst
Copy link
Collaborator Author

SophieHerbst commented Mar 22, 2024

Looking into the code, the stats shouldn't have been run if there is only one subject:

Screenshot 2024-03-22 at 12 02 55

@larsoner
Copy link
Member

Looking into the code, the stats shouldn't have been run if there is only one subject:

In this case we should maybe make sure to remove the section if it's present:

https://mne.tools/dev/generated/mne.Report.html#mne.Report.remove

To decide if we need to remove it or not we probably need mne-tools/mne-python#12424

I'll reopen an re-title since it seems like there is an issue to be fixed still

@larsoner larsoner reopened this Mar 25, 2024
@larsoner larsoner changed the title ENH: systematically add subject count to sub-average report BUG: report can contain stale stats when one subject is used Mar 25, 2024
@larsoner
Copy link
Member

I realized this problem is more widespread than we thought. For example if you run once with decoding enabled in your config, then run again with it disabled (perhaps with some other changes like filtering or whatever), that original decoding report from potentially long ago when you ran with it enabled will still be in the report.

Same thing for stuff like autobad maxwell report stuff, etc. -- enable it then disable it and the info will still show up in the report. Worse there the TSV with the bads won't get overwritten so you'll be stuck with whatever autobads it found 😨

Same thing with if you switch between SSP and ICA I think (untested but I think it's the case).

So the safest thing I think is to never use the Skipping ... like we do now, and have a conditional inside the step itself that always either adds to the report section(s) or removes them (and maybe some output files in some cases?) depending on whether or not the step is used.

@hoechenberger
Copy link
Member

Wow, omg…

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

No branches or pull requests

3 participants