Skip to content

Conversation

@christianbrodbeck
Copy link
Member

@christianbrodbeck christianbrodbeck commented Jun 27, 2016

Related to #3311.

The builtin KIT-157 layout is based on the KIT-NY system and does not apply for KIT-UMD data. This PR distinguishes the two systems based on sensor positions. If in the future the KIT system-identifier is stored in the measurement info this distinction will be more efficient, but the sensor-based distinction will always be useful for legacy FIFF files.

@dengemann
Copy link
Member

Give a sign once it's ready.

@christianbrodbeck
Copy link
Member Author

This is ready as a temporary fix that makes plotting work for UMD data.

@christianbrodbeck
Copy link
Member Author

The travis problem is just the FAIL: Doctest: mne.filter.construct_iir_filter

@christianbrodbeck christianbrodbeck changed the title FIX find_layout() for KIT-UMD data [MRG] FIX find_layout() for KIT-UMD data Jun 29, 2016
@agramfort
Copy link
Member

doesn't it break/change the code of people's code using KIT at NYU?

layout_name = 'KIT-157'
# This applies to the KIT systems at NYU and UMD which have different
# layouts. The 'KIT-157' layout applies to the NYU system.
return None
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't look right. Like this it would never find 'KIT-157'. It's still in use somewhere, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

You can still manually specify it.

@christianbrodbeck
Copy link
Member Author

@agramfort it will change the looks of topographic plots slightly because the layout will be slightly different. To avoid that (and still make it work for UMD data) I would have to add the proposed guessing of system by sensor positions.

@christianbrodbeck christianbrodbeck changed the title [MRG] FIX find_layout() for KIT-UMD data [WIP] FIX find_layout() for KIT-UMD data Jun 29, 2016
@christianbrodbeck
Copy link
Member Author

Actually I noticed not all topographic plotting functions currently resort to the automatic layout, so we'll need a more involved fix for this...

@christianbrodbeck christianbrodbeck changed the title [WIP] FIX find_layout() for KIT-UMD data [WIP] FIX Topographic plotting for KIT-UMD data Jun 29, 2016
For KIT systems with <=157 channels, create layout based on sensor
position
@agramfort
Copy link
Member

agramfort commented Jun 30, 2016 via email

@codecov-io
Copy link

codecov-io commented Jul 1, 2016

Current coverage is 86.66%

Merging #3349 into master will decrease coverage by 0.01%

@@             master      #3349   diff @@
==========================================
  Files           335        335          
  Lines         57648      57660    +12   
  Methods           0          0          
  Messages          0          0          
  Branches       8785       8787     +2   
==========================================
+ Hits          49967      49969     +2   
- Misses         5002       5011     +9   
- Partials       2679       2680     +1   

Sunburst

Powered by Codecov. Last updated by abbc097...7389054

@christianbrodbeck christianbrodbeck changed the title [WIP] FIX Topographic plotting for KIT-UMD data [MRG] FIX Topographic plotting for KIT-UMD data Jul 1, 2016
@christianbrodbeck
Copy link
Member Author

This new solution should not break any old code, and can be updated to be more efficient once we have KIT System-IDs in the measurement info.

elif n_kit_grads > 0:
layout_name = _find_kit_system(info, n_kit_grads)
if layout_name == 'KIT-UMD':
return _auto_layout(info)
Copy link
Member

Choose a reason for hiding this comment

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

why not adding a layout file for KIT-UMD? by doing this we could share the file between software.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ho would I create a layout file, using the generate_2d_layout function? And what are the criteria for a good layout? That plot_topo looks good?

Copy link
Member

Choose a reason for hiding this comment

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

if generate_2d_layout output looks good with plot_topo yes.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, I generated an automatic layout and then I nudged the rectangles around manually to minimize overlap, which seemed preferable for plot_topo. I noticed though that topomap also uses the same layout where a deterministic arrangement seems preferable?

kit1
kit4

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

There is some, but limited symmetry... here is a raw.plot_sensors() plot and a layout generated with MNE _auto_topomap_coords():

sensors

auto

Copy link
Member

Choose a reason for hiding this comment

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

ok indeed. Then I think you're manually edited version is good enough.

@christianbrodbeck
Copy link
Member Author

@agramfort I added a UMD layout. It will be easier to add a test via the KIT_SYSTEM_ID PR because it adds a small UMD raw file.

@agramfort agramfort merged commit 5d33ad2 into mne-tools:master Jul 5, 2016
@agramfort
Copy link
Member

thx @christianbrodbeck

@christianbrodbeck christianbrodbeck deleted the umd-layout branch July 5, 2016 12:25
jona-sassenhagen added a commit to jona-sassenhagen/mne-python that referenced this pull request Jul 5, 2016
* 'master' of git://github.com/mne-tools/mne-python: (48 commits)
  FIX: Flake
  fix pep8
  [MRG] FIX Topographic plotting for KIT-UMD data (mne-tools#3349)
  QUICKFIX: logging test
  Error message for simulate evoked. (mne-tools#3372)
  Warnings for big bems. (mne-tools#3374)
  Add test for baseline correction.
  Use apply_baseline in read_evokeds.
  Makes (None, 0) the default value baseline in apply_baseline.
  [FIX] Adds apply_baseline to Evoked
  fix component inds in ica tutorial (mne-tools#3379)
  FIX: Xdawn with shuffled epochs (mne-tools#3373)
  FIX consistency:  Epochs.load_data() should always return self (mne-tools#3376)
  MRG: Build relevant examples (mne-tools#3365)
  [MRG] set vmin, vmax after smoothing in plot_epochs_image (mne-tools#3360)
  [FIX] Attempt to fix circle.
  fixing a bg plotting bug for topo plots
  Modified test and tutorial.
  [MRG] convert surface using python. (mne-tools#3273)
  [MRG] raw.plot_psd() with flat channel (mne-tools#3342)
  ...
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.

5 participants