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

WIP [FIX] BabyMeg HPI Fitting #800

Closed
wants to merge 3 commits into from
Closed

Conversation

RDoerfel
Copy link
Contributor

PR #773 resubmitted.

This PR tries to fix the ongoing problems with the BabyMeg HPI fitting. After quite a lot of debugging and desperate nights, I was able to fix some stuff and get at least stable fits, also with a 'high' error. The fits are quite stable around 10 mm displacement error.

Nevertheless, this is still more than with the old 42ce89b version before any changes were introduced. Besides a lot of other things, I also ended up copying the entire old HPI class and embed it into the current version. The results were as bad as with the most recent version. From that, I conclude that the problem was introduced with the new HPI plugin and I would suggest checking what is the difference between the way the HPI plugin and former rthpis and HPIView.

This brings us to what is yet in this PR:

HPI Fitting Class:

  • use absolute value to determine channel with maximum amplitude

HPI Plugin:

  • projectors and compensators were never updated and applied not entirely correct. The way now is more similar to how it was done before.

That's it so far. I'm still not sure what will fix it entirely. It must be something that only affects the BabyMeg, since the VectorView fits are running like a charm. Also, the good results with the current versions are only obtained using the advanced model, and not the fast version.

@codecov-commenter
Copy link

codecov-commenter commented May 29, 2021

Codecov Report

Merging #800 (3c216ce) into main (cfa4bba) will increase coverage by 0.05%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #800      +/-   ##
==========================================
+ Coverage   29.47%   29.52%   +0.05%     
==========================================
  Files         449      450       +1     
  Lines       39179    39296     +117     
==========================================
+ Hits        11548    11603      +55     
- Misses      27631    27693      +62     
Impacted Files Coverage Δ
libraries/inverse/hpiFit/hpifit.cpp 82.53% <100.00%> (ø)
libraries/connectivity/network/networknode.h 0.00% <0.00%> (-100.00%) ⬇️
libraries/fs/label.h 50.00% <0.00%> (-50.00%) ⬇️
libraries/fiff/fiff_stream.h 50.00% <0.00%> (-50.00%) ⬇️
applications/mne_edf2fiff/edf_info.h 0.00% <0.00%> (-15.39%) ⬇️
libraries/utils/mnemath.h 6.45% <0.00%> (-0.45%) ⬇️
libraries/mne/mne_forwardsolution.h 11.42% <0.00%> (-0.34%) ⬇️
libraries/disp/viewers/triggerdetectionview.cpp 0.73% <0.00%> (-0.01%) ⬇️
libraries/fs/surface.h 0.00% <0.00%> (ø)
libraries/fs/colortable.h 0.00% <0.00%> (ø)
... and 17 more

@RDoerfel RDoerfel changed the title Debug bm [WIP,FIX] BabyMeg HPI Fitting Jun 2, 2021
@juangpc juangpc changed the title [WIP,FIX] BabyMeg HPI Fitting WIP [FIX] BabyMeg HPI Fitting Aug 18, 2021
@juangpc
Copy link
Collaborator

juangpc commented Oct 22, 2021

@RDoerfel could you have a look at this conflicts? if not, let me know please. Thx

@RDoerfel
Copy link
Contributor Author

@RDoerfel could you have a look at this conflicts? if not, let me know please. Thx

I think this PR will be obsolete once I refactored the HPI fitting. I'll close it once I'm there.

@juangpc
Copy link
Collaborator

juangpc commented Oct 26, 2021

Ok. No rush.

@RDoerfel
Copy link
Contributor Author

obsolete with #884

@RDoerfel RDoerfel closed this Feb 25, 2022
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.

3 participants