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

Fix events sorting bug and clarify some documentation #56

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

wronk
Copy link

@wronk wronk commented Apr 21, 2017

No description provided.

@@ -43,15 +43,14 @@
src_outputs = hcp.anatomy.compute_forward_stack(
subject=subject, subjects_dir=subjects_dir,
hcp_path=hcp_path, recordings_path=recordings_path,
# speed up computations here. Setting `add_dist` to True may improve the
# accuracy.
# Speed up computations here by setting `add_dist` to False.
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Eric89GXL, IIRC, you added distances for some specialized src spaces analysis, right? It doesn't actually improve accuracy, correct?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The add_dists also adds patch info which is used in forward computation to make things slightly better

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, good to know. I'll add that comment back in

@wronk
Copy link
Author

wronk commented Apr 21, 2017

@dengemann, do I need to rerun all the example/tutorial analyses on my end?

@dengemann
Copy link
Member

dengemann commented Apr 21, 2017 via email

@wronk
Copy link
Author

wronk commented Apr 21, 2017

I'm hadn't been analyzing the WM data, so I'll download it and get back to you

@wronk
Copy link
Author

wronk commented Apr 24, 2017

Hmm, sorting events when processing the data from scratch made the correlation with the fully-processed HCP data go down (~0.7 to ~0.4). This is definitely not what I was expecting. Ideas?

figure_1-2_postfix

@wronk
Copy link
Author

wronk commented Jun 23, 2017

Hi @dengemann :)

@wronk
Copy link
Author

wronk commented Oct 29, 2017

Bonjour @dengemann :)

@dengemann
Copy link
Member

dengemann commented Oct 29, 2017 via email

@wronk
Copy link
Author

wronk commented Oct 29, 2017

I don't think I'll have much time to work in this (starting a new job :D), but I would suggest merging this asap. I don't know why the examples get slightly worse, but the bugfix for plot_reproduce_erf.py is important. Anyone who uses that code will have their analysis pipeline destroyed because the columns in the events array are currently all being sorted independently. End result is that event timings will no longer line up with the time point of the raw file.

@dengemann
Copy link
Member

dengemann commented Oct 29, 2017 via email

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