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.

Sign up for GitHub

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

Jump to bottom

Change how surrogate histograms are calculated #63

Merged
merged 10 commits into from
Feb 15, 2022
Merged

Change how surrogate histograms are calculated #63

merged 10 commits into from
Feb 15, 2022

Conversation

eurunuela
Copy link
Contributor

Closes #53 .

Changes proposed in this pull request:

  • Remove second calculation of surrogate matrices.
  • Calculate histograms of surrogate matrices independently and then sum.

@eurunuela eurunuela mentioned this pull request Feb 14, 2022
Closed
@eurunuela eurunuela mentioned this pull request Feb 15, 2022
Closed
@eurunuela eurunuela marked this pull request as ready for review February 15, 2022 10:37
@eurunuela eurunuela requested a review from vinferrer February 15, 2022 10:37
vinferrer
vinferrer reviewed Feb 15, 2022
View reviewed changes
connPFM/connectivity/connectivity_utils.py Outdated
percentile=95,
def sum_histograms(
hist_list,
numrand,
Copy link
Member

Choose a reason for hiding this comment

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

You could avoid using this variable by extracting the number of surrogates as len(hist_list) and probably the same with nbins

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in 1526086.

@eurunuela eurunuela requested a review from vinferrer February 15, 2022 11:08
vinferrer
vinferrer approved these changes Feb 15, 2022
View reviewed changes
Copy link
Member

@vinferrer vinferrer left a comment

Choose a reason for hiding this comment

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

LGTM!!

@eurunuela eurunuela merged commit b4d37af into SPiN-Lab:main Feb 15, 2022
@eurunuela eurunuela deleted the surrogate_histograms branch February 15, 2022 11:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vinferrer vinferrer vinferrer approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Calculate thresholds independently for each surrogate
2 participants
@eurunuela @vinferrer