-
Notifications
You must be signed in to change notification settings - Fork 71
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
ENH: Minor fixes to add #609
Comments
@SophieHerbst had an aborted epochs run; after restarting the step, it seems that epochs for all subjects were being re-created – not only those for subjects that were not processed before. Is that a known limitation? |
I'm surprised that happens since the subjects are looped over. That does sound like a bug |
I believe she had debug mode turned on, i.e., |
To test without debugging mode, I rerun the preprocessing using --step=preprocessing. Here is the log with the time stamps: Output
|
Ouch, that's bad! These should take < 1 sec not tens of seconds. Have you ever done line profiling with
and when execution finishes it will print out line-by-line the time spent doing each thing. Perhaps this would be a good spot to add the mne-bids-pipeline/mne_bids_pipeline/_run.py Line 140 in 59c8343
If that doesn't work I can look into refactoring |
... and @SophieHerbst can you make sure you're on latest |
@larsoner We should cut a new release with those fixes. As far as I know, @SophieHerbst uses the latest release, installed following our instructions on the website |
@hoechenberger feel free. Or if you don't have time I can do it. I think nowadays we just cut a release on GitHub with an appropriate tag |
@larsoner I tagged a new release: Waiting for it to land on PyPI and conda-forge. |
@larsoner updating from 1.1 to 1.2 solved it. MUCH faster now! |
But it now runs the frequency filtering again, which has been run before
|
Have you waited for it to finish, then re-run it? I'd expect it to re-run some stuff once between mne-bids-pipeline version jumps |
Not sure what you mean. I had aborted the previous command that was taking forever, and relaunched with --step=preprocessing after the update. |
You shouldn't have to. But I wouldn't be surprised if some steps re-ran once (seemingly unnecessarily) after an update. That's why I was trying to ask if you tried running twice after the update -- I'd expect a re-run once but the second time you run it it should cache. If it doesn't, it seems like a bug. But to be safest I'd just re-run the whole pipeline after an update. Then everything should be up to date... |
ok, doing first complete re-run after the update. I will try a second time after and keep you posted. |
While I am still waiting for the re-run to finish (I have a lot of runs): it would be helpful if the version of the bids-pipeline used per step was to be found in the output for reporting in a paper and replicability. Is this currently the case? |
It should show up in the mne sys info part of the report IIRC |
I see the MNE version there, but not the version of the bids-pipeline |
@larsoner I confirm: first run after update re-runs some steps, 2nd run after update does not, cashing is fast as expected |
I had to take a double-take here as I kept reading "crashing as fast as expected" 😅😂 |
Just so I don't forget, from #607
freesurfer/_02_coreg_surfaces.py
: ENH: Cache coreg, bem, cov #615_01_make_bem_surfaces.py
steps by dealing with recomputation params properly: ENH: Cache coreg, bem, cov #615_03_make_cov.py
: ENH: Cache coreg, bem, cov #615_02_make_forward.py
by splittingtrans
step out separately (maybe?): MAINT: Proposed minor reorg #616, MAINT: Reorganize modules and cache #618_04_make_inverse.py
: MAINT: Reorganize modules and cache #618config.*
use inside functions, or at least ensure caching is not broken with them: MAINT: Encapsulate config import #635return_candidates=True
infind_empty_room
if available: It should be easier to see at a single glance how many epochs have been rejected, both on individual and on group/study level #524 BUG: Fix bug with empty-room matching message #642source/_99_group_average.py
: ENH: Cache grand-average steps #765sensor/_99_group_average.py
: ENH: Cache grand-average steps #765The text was updated successfully, but these errors were encountered: