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

DOC - document the units of the inverse operation for eLORETA #2231

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

robertoostenveld
Copy link
Contributor

  • added check that the user did not specify leadfield options in case the input already contains the leadfield
  • added check that the user did not specify a regularization parameter in case the input already contains the filter

- added check that the user did not specify leadfield options in case the input already contains the leadfield
- added check that the user did not specify a regularization parameter in case the input already contains the filter
@robertoostenveld robertoostenveld changed the title document the units of the inverse operation for eLORETA DOC - document the units of the inverse operation for eLORETA Apr 21, 2023
@robertoostenveld
Copy link
Contributor Author

This follows from a conversation with @mcpiastra. The page https://www.fieldtriptoolbox.org/faq/units/ is related, as are the occasional questions on the mailing list such as https://mailman.science.ru.nl/pipermail/fieldtrip/2021-June/040932.html

It would be nice to have the units in general better documented. Documenting them at the lowest accessible level (i.e. ft_inverse_xxx) seems feasible. Documenting them at a higher level is more difficult, since there can be so many choices in the analysis pipeline that affect this. What might be good to document generally though is that inverse computations are not arbitrary if all data, geometrical objects and models (like the leadfield) are specified in SI units.

@robertoostenveld
Copy link
Contributor Author

@schoffelen can I ask you to review this content-wise? I will make sure to regression-test the checks for the leadfield and filter options being empty before it gets merged.

@mcpiastra
Copy link
Contributor

hi @robertoostenveld and @schoffelen ,
there are 10 left ft_inverse_something where I would now add checks for the leadfield. is one PR for all functions a good strategy?
thanks,
MC

@robertoostenveld
Copy link
Contributor Author

is one PR for all functions a good strategy?

Yes, that would be fine. Probably the changes will be very similar to all functions, so that facilitates review.

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 this pull request may close these issues.

None yet

2 participants