-
Notifications
You must be signed in to change notification settings - Fork 14
adding alternative mode volume calculation for lower memory usage #287
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
Conversation
|
Spell check passed successfully for 1 notebook(s). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @FilipeFcp looks good to me overall. Only thing I'm wondering is whether the interval_space change was necessary? It seems like you changed from coarse spatial sampling to coarse time-domain sampling, with a net memory decrease of a factor of 2, which sounds reasonable but I'm wondering keeping the coarser spatial sampling would change the results significantly?
tomflexcompute
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's great @FilipeFcp . Seems like there is a 10-20% difference in the calculated mode volume. Are there results in the literature where we can benchmark the results?
|
Thanks @yaugenst-flex It doesnt change significantly, but it brings the results closer to those reported in the paper. For this type of calculation, I believe the error introduced by using the raw data is preferable to the loss of data caused by downsampling. I added a comment regarding the interval_space in the notebook. |
|
@tomflexcompute Yes, the structure is from this paper. They report 0.39 (lda/n)^3 |
Great that's very good! |
|
@FilipeFcp if you could squash these commits it’s good to go |
Hi all.
Since a user used this notebook as a reference and had issues with kernel crashing during mode volume calculation, I made a small modification to reduce the memory usage, and also added a small discussion on how to use raw monitor data (without expanding symmetries) to calculate it.