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

STC saturated – rank problem? #905

Closed
SophieHerbst opened this issue Mar 27, 2024 · 11 comments · Fixed by #919
Closed

STC saturated – rank problem? #905

SophieHerbst opened this issue Mar 27, 2024 · 11 comments · Fixed by #919

Comments

@SophieHerbst
Copy link
Collaborator

SophieHerbst commented Mar 27, 2024

I am not happy with the stcs of some of my subjects, they look like the screenshot attached: at one moment in time there is very high activity everywhere, which messes up the scale. I remember that a similar outcome was caused by differences in rank between the cov used for whitening, and the rank given to compute the inverse.

With cfg.noise_cov = 'rest', the following should read the info of the resting state run to compute the inverse.

l. 52 in source/_05_make_inverse.py
in_files["info"] = bids_path.copy().update(**cfg.source_info_path_update)

I cannot figure out what the **cfg.source_info_path_update does.
Thanks for any help!

Screenshot 2024-03-27 at 17 12 03
@larsoner
Copy link
Member

larsoner commented Mar 27, 2024

Yes it's probably a rank mismatch between the empty room resting state and task data. Are you using movement compensation? If subjects move a lot it could be an issue with our imperfect/incomplete capturing of movements during runs compared to empty rooms. Or you could be hitting #760.

A long-term plan for at least some of this stuff could come out of mne-tools/mne-python#4676, but even with that I don't think we're guaranteed to solve all issues.

I've been meaning to add something that allows computing the rank robustly from the epochs of interest -- in my work with infants who move a lot I almost always use something like:

rank = mne.compute_rank(epochs, tol=1e-3, tol_kind="relative")

and it has worked well. I propose we add something like:

rank_kind: Union[Literal["info"], Dict[str, Any]]

where the backward-compatible rank_kind = "info" is what we do currently (guess as best we can from info), otherwise allow a dict which is a set of kwargs to pass to mne.compute_rank(epochs, **rank_kind). So in your case to follow my suggestion you would just add to your config:

rank_kind = dict(tol=1e-3, tol_kind="relative")

and then this would compute the rank from epochs and also pass rank=... to the covariance, inverse, and (possibly) decoding code.

@SophieHerbst
Copy link
Collaborator Author

thanks for the fast reply @larsoner!
would your solution still work if the rank of the resting state run is lower than that of the epochs (because there was one more bad sensor for instance)?

in my own code I did the following to prevent the problem:

  • compute covariance from resting state with rank=info
  • when computing the inverse, load the resting state again and pass its info (not the one of the epochs) to compute_inverse

@larsoner
Copy link
Member

Ahh yes in principle you should probably compute the rank over whatever you're using to compute the covariance. So it should be computed in the compute_covariance step probably using whatever data you're computing the covariance over, which could be rest, empty room or task epochs. And in the case of a custom covariance note for people that the rank will be computed over the returned covariance which is generally less numerically accurate (but probably okay).

@SophieHerbst
Copy link
Collaborator Author

SophieHerbst commented Mar 29, 2024

@larsoner it seems already the case that one can chose the file from which to read the info in source/05_make_inverse.py
l. 51: in_files["info"] = bids_path.copy().update(**cfg.source_info_path_update)

After setting source_info_path_update in config, my stcs look fine.

noise_cov = 'rest'
source_info_path_update = {'processing': 'clean',
                            'suffix': 'raw',
                            'task': noise_cov}

Is this the intended usage?
If yes, wouldn't it make more sense to automatically use the info from the file from which the cov was computed?

@larsoner
Copy link
Member

Yeah it seems like that would make sense. Not sure what the current behavior / default is for source_info_path_update is, seems like it should use the noise_cov if it can

@hoechenberger
Copy link
Member

Not sure what the current behavior / default is for source_info_path_update is, seems like it should use the noise_cov if it can

It reads the Info from the Evoked

@SophieHerbst
Copy link
Collaborator Author

I agree. I couldn't figure out what the default behavior does.

@hoechenberger
Copy link
Member

So the solution would be to NOT use the Info from the Evoked if the noise cov was estimated from empty-room or resting-state data, do I understand this correctly?

@SophieHerbst
Copy link
Collaborator Author

yes, I think this is what causes the mismatch in ranks and the weird stcs

@SophieHerbst
Copy link
Collaborator Author

@hoechenberger if you can point me to where this happens, I can try to change it

@hoechenberger
Copy link
Member

@hoechenberger if you can point me to where this happens, I can try to change it

Sorry, I can't give any useful advice for now, as I'm busy with other stuff … maybe by the end of the week, but I cannot promise anything!

larsoner added a commit that referenced this issue Apr 2, 2024
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Eric Larson <[email protected]>
larsoner added a commit to larsoner/mne-bids-pipeline that referenced this issue Apr 16, 2024
* upstream/main:
  change default for info to use for inverse mne-tools#905 (mne-tools#919)
  Improve documentation and config validation of `loose` and `depth` parameters; drop support for `loose=None` (mne-tools#915)
  enhance documentation of caching, continuation of mne-tools#914 (mne-tools#918)
  [pre-commit.ci] pre-commit autoupdate (mne-tools#917)
  Restructure configuration options documentation sections (mne-tools#914)
  Try to fix documentation deployment (mne-tools#913)
  Do not show `Annotated` types in configuration options documentation (mne-tools#911)
  Add number of subjects to grand-average report (cont'd) (mne-tools#910)
  MAINT: Ensure input changes cause output changes (mne-tools#904)
  Render type annotations in the documentation again (mne-tools#909)
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

Successfully merging a pull request may close this issue.

3 participants