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

MRG, ENH: Speed up epochs.copy #7968

Merged
merged 3 commits into from
Jul 7, 2020
Merged

MRG, ENH: Speed up epochs.copy #7968

merged 3 commits into from
Jul 7, 2020

Conversation

larsoner
Copy link
Member

@larsoner larsoner commented Jul 6, 2020

Closes #7947

On master the snippet here I get:

Time untouched epochs:    0:00:00.014442
Time concatenated epochs: 0:00:00.457064

On this PR:

Time untouched epochs:    0:00:00.014059
Time concatenated epochs: 0:00:00.022924

So it's still ~2x slower, which I guess is to be expected because selection does grow by 2x. This should at least be hopefully acceptable compared to the ~30x slower it is on master.

@drammock
Copy link
Member

drammock commented Jul 6, 2020

relevant failure:

mne\viz\tests\test_epochs.py:272: in test_plot_drop_log
    plot_drop_log([['One'], [], []])
mne\viz\epochs.py:603: in plot_drop_log
    percent = _drop_log_stats(drop_log, ignore)
mne\epochs.py:1923: in _drop_log_stats
    raise ValueError('drop_log must be a tuple of tuples')
E   ValueError: drop_log must be a tuple of tuples

@larsoner
Copy link
Member Author

larsoner commented Jul 6, 2020

Fixed by fc85ca7, appears to be coming back green

drammock
drammock approved these changes Jul 6, 2020
View reviewed changes
mne/epochs.py Outdated Show resolved Hide resolved
agramfort
agramfort approved these changes Jul 7, 2020
View reviewed changes
Copy link
Member

@agramfort agramfort left a comment

Choose a reason for hiding this comment

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

@drammock merge if happy

there is your last question to answer

@larsoner larsoner merged commit c53ce68 into mne-tools:master Jul 7, 2020
@larsoner larsoner deleted the epo branch July 7, 2020 12:27
larsoner added a commit to larsoner/mne-python that referenced this pull request Jul 8, 2020
@larsoner
Merge remote-tracking branch 'upstream/master' into refactor
b60f9b0 
* upstream/master: (30 commits)
  MRG: Add remove_labels to _Brain (mne-tools#7964)
  Add get_picked_points (mne-tools#7963)
  ENH: Add OpenGL info to mne sys_info (mne-tools#7976)
  [MRG] Fix reject_tmin and reject_tmax for reject_by_annotation in mne.Epochs (mne-tools#7967)
  mrg: Add scalar mult and div operators for AverageTFR (mne-tools#7957)
  MRG, MAINT: Cleaner workaround for Sphinx linking issue (mne-tools#7970)
  MRG, ENH: Speed up epochs.copy (mne-tools#7968)
  MRG, BUG: Allow ref mags to have a comp grade (mne-tools#7965)
  do not forget to pass adjacency (mne-tools#7961)
  [MRG] fix Issue with stc.project after restricting to a label (mne-tools#7950)
  Only process nirx event file if present (mne-tools#7951)
  MRG+1: BUG: info['bads'] order shouldn't matter in write_evokeds() (mne-tools#7954)
  Fix some small glitches introduced via mne-tools#7845 (mne-tools#7952)
  Add time player (mne-tools#7940)
  MAINT: Clean up VTK9 offset array [circle front] (mne-tools#7953)
  MAINT: Skip a few more on macOS (mne-tools#7948)
  fix links [skip travis] (mne-tools#7949)
  MRG, MAINT: Tweak CIs (mne-tools#7943)
  MRG, BUG: Fix vector scaling (mne-tools#7934)
  MRG, VIZ, BUG: handle CSD channel type when topo plotting (mne-tools#7935)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@agramfort agramfort agramfort approved these changes

@drammock drammock drammock 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.

Extremely slow access to epochs after channel remapping.
3 participants
@larsoner @drammock @agramfort