Skip to content

Conversation

@larsoner
Copy link
Member

@larsoner larsoner commented Nov 2, 2023

Okay @nordme on this branch if you do:

MNE_COREG_HEAD_OPACITY=0.25 mne coreg --subjects-dir=~/mne_data/MNE-sample-data/subjects -s sample -f ~/mne_data/MNE-sample-data/MEG/sample/sample_audvis_raw.fif --trans ~/mne_data/MNE-sample-data/MEG/sample/sample_audvis_raw-trans.fif

you should see:

Screenshot 2023-11-02 at 11 49 58 AM

You can see the points inside, as well as scale-by-distance working again (it wasn't before).

Closes #12123

@larsoner larsoner added this to the 1.6 milestone Nov 2, 2023
@nordme
Copy link
Contributor

nordme commented Nov 2, 2023

@larsoner Hmm, I'm feeling a bit mystified. Running the above command, I get this.
Screenshot from 2023-11-02 12-07-29

It looks like I'm on an old version of coreg, but I'm in my dev environment, your branch is checked out and here's the git log showing your latest commit.

`commit 42f8542 (HEAD -> l_coreg2, larsoner/coreg)
Author: Eric Larson larson.eric.d@gmail.com
Date: Thu Nov 2 11:46:02 2023 -0400

FIX: Fix bug with coreg scalars

which mne` and mne sys_info output look correct, and MNE_COREG_SCALE_WITH_DISTANCE=true, so why does it look like my command is still running an old version of coreg?

@larsoner
Copy link
Member Author

larsoner commented Nov 2, 2023

Can you try modifying mne/viz/_3d.py so that _plot_glyphs raises an error like a 1 / 0 or something, then run the above command? That would confirm whether or not mne coreg is actually using that code. But in principle doing which mne should tell you which binary is being used and mne sys_info -p should use the same command and import.

@nordme
Copy link
Contributor

nordme commented Nov 2, 2023

@larsoner Yeah, if I add a RuntimeError into _plot_glyphs nothing happens. Here's my which mne output:

(sprint_2023) erica@uberica:~/repos/mne-python$ which mne
/home/erica/miniconda3/envs/sprint_2023/bin/mne

And my mne sys_info output:

Platform             Linux-6.2.0-35-generic-x86_64-with-glibc2.35
Python               3.11.5 | packaged by conda-forge | (main, Aug 27 2023, 03:34:09) [GCC 12.3.0]
Executable           /home/erica/miniconda3/envs/sprint_2023/bin/python3.1
CPU                  x86_64 (24 cores)
Memory               62.7 GB

Core
├☑ mne               1.6.0.dev88+gb9e907667
├☑ numpy             1.26.0 (unknown linalg bindings (threadpoolctl module not found: No module named 'threadpoolctl'))
├☑ scipy             1.11.2
├☑ matplotlib        3.8.0 (backend=QtAgg)
├☑ pooch             1.7.0
└☑ jinja2            3.1.2

Numerical (optional)
├☑ nibabel           5.1.0
├☑ dipy              1.7.0
├☑ pandas            2.1.1
└☐ unavailable       sklearn, numba, nilearn, openmeeg, cupy

Visualization (optional)
├☑ pyvista           0.42.2 (OpenGL 4.6 (Core Profile) Mesa 23.0.4-0ubuntu1~22.04.1 via AMD Radeon RX 570 Series (polaris10, LLVM 15.0.7, DRM 3.49, 6.2.0-35-generic))
├☑ pyvistaqt         0.11.0
├☑ vtk               9.2.6
Warning: Ignoring XDG_SESSION_TYPE=wayland on Gnome. Use QT_QPA_PLATFORM=wayland to run on Wayland anyway.
├☑ qtpy              2.4.0 (PyQt5=5.15.2)
└☐ unavailable       ipympl, pyqtgraph, mne-qt-browser, ipywidgets, trame_client, trame_server, trame_vtk, trame_vuetify

Ecosystem (optional)
├☑ mne-bids          0.14.dev0
├☑ mne-connectivity  0.6.0dev0
└☐ unavailable       mne-nirs, mne-features, mne-icalabel, mne-bids-pipeline


@larsoner
Copy link
Member Author

larsoner commented Nov 2, 2023

Yeah, if I add a RuntimeError into _plot_glyphs nothing happens.

Okay I take this as good news actually as it suggests that my fix in this PR has a chance of working still :)

What is the path for mne when you do mne sys_info -p? For example for me it's:

├☑ mne               1.6.0.dev235+gc154d8aa7 (devel, latest release is 1.5.1)
│                    /Users/larsoner/python/mne-python/mne

and /Users/larsoner/python/mne-python is the one I use with git. I'm guessing/hoping that the path you see here is not the one you used to check out my branch. And then a pip install -ve . would make your current env start to use the dev / git version...

@larsoner
Copy link
Member Author

larsoner commented Nov 2, 2023

... oh and it might matter whether you execute that mne sys_info -p from inside mne-python/ git checkout (because it has a mne/ subdirectory) or someplace else like home / ~.

@nordme
Copy link
Contributor

nordme commented Nov 3, 2023

$ mne sys_info -p
Platform             Linux-6.2.0-35-generic-x86_64-with-glibc2.35
Python               3.11.5 | packaged by conda-forge | (main, Aug 27 2023, 03:34:09) [GCC 12.3.0]
Executable           /home/erica/miniconda3/envs/sprint_2023/bin/python3.1
CPU                  x86_64 (24 cores)
Memory               62.7 GB

Core
├☑ mne               1.6.0.dev88+gb9e907667/home/erica/miniconda3/envs/sprint_2023/lib/python3.11/site-packages/mne
├☑ numpy             1.26.0 (unknown linalg bindings (threadpoolctl module not found: No module named 'threadpoolctl'))
│                    /home/erica/miniconda3/envs/sprint_2023/lib/python3.11/site-packages/numpy
├☑ scipy             1.11.2/home/erica/miniconda3/envs/sprint_2023/lib/python3.11/site-packages/scipy
├☑ matplotlib        3.8.0 (backend=QtAgg)
│                    /home/erica/miniconda3/envs/sprint_2023/lib/python3.11/site-packages/matplotlib
├☑ pooch             1.7.0/home/erica/miniconda3/envs/sprint_2023/lib/python3.11/site-packages/pooch
└☑ jinja2            3.1.2
                     /home/erica/miniconda3/envs/sprint_2023/lib/python3.11/site-packages/jinja2

Numerical (optional)
├☑ nibabel           5.1.0/home/erica/miniconda3/envs/sprint_2023/lib/python3.11/site-packages/nibabel
├☑ dipy              1.7.0/home/erica/miniconda3/envs/sprint_2023/lib/python3.11/site-packages/dipy
├☑ pandas            2.1.1/home/erica/miniconda3/envs/sprint_2023/lib/python3.11/site-packages/pandas
└☐ unavailable       sklearn, numba, nilearn, openmeeg, cupy

Visualization (optional)
├☑ pyvista           0.42.2 (OpenGL 4.6 (Core Profile) Mesa 23.0.4-0ubuntu1~22.04.1 via AMD Radeon RX 570 Series (polaris10, LLVM 15.0.7, DRM 3.49, 6.2.0-35-generic))
│                    /home/erica/miniconda3/envs/sprint_2023/lib/python3.11/site-packages/pyvista
├☑ pyvistaqt         0.11.0/home/erica/miniconda3/envs/sprint_2023/lib/python3.11/site-packages/pyvistaqt
├☑ vtk               9.2.6/home/erica/miniconda3/envs/sprint_2023/lib/python3.11/site-packages
Warning: Ignoring XDG_SESSION_TYPE=wayland on Gnome. Use QT_QPA_PLATFORM=wayland to run on Wayland anyway.
├☑ qtpy              2.4.0 (PyQt5=5.15.2)
│                    /home/erica/miniconda3/envs/sprint_2023/lib/python3.11/site-packages/qtpy
└☐ unavailable       ipympl, pyqtgraph, mne-qt-browser, ipywidgets, trame_client, trame_server, trame_vtk, trame_vuetify

Ecosystem (optional)
├☑ mne-bids          0.14.dev0/home/erica/miniconda3/envs/sprint_2023/lib/python3.11/site-packages/mne_bids
├☑ mne-connectivity  0.6.0dev0/home/erica/miniconda3/envs/sprint_2023/lib/python3.11/site-packages/mne_connectivity
└☐ unavailable       mne-nirs, mne-features, mne-icalabel, mne-bids-pipeline

@larsoner I think you're right that your fix probably works, and my environment is not pulling it in properly. The path to my mne-python clone is /home/erica/repos/mne-python, and that's not what mne sys_info -p is showing under the mne line. However, I already tried redoing the editable pip install on a clone of this environment and that broke all the mne terminal commands completely, so let's troubleshoot my environment via Slack and then I'll touch base on this PR once we know I'm actually testing your fix!!! :) :)

@nordme
Copy link
Contributor

nordme commented Nov 3, 2023

Alright, @larsoner and I discovered that there was a problematic empty /mne dir in my environment's site-packages that pip failed to remove during the editable install. Manual removal of that directory got us back up and running. With that fix, I replicate the correct behavior.
Screenshot from 2023-11-03 10-24-29

Looks good!!

@drammock drammock merged commit ec87fd8 into mne-tools:main Nov 3, 2023
@drammock drammock deleted the coreg branch November 3, 2023 20:17
@drammock
Copy link
Member

drammock commented Nov 3, 2023

awesome, thanks @larsoner and @nordme for persistence in tracking down the error

larsoner added a commit to JD-Zhu/mne-python that referenced this pull request Nov 3, 2023
* upstream/main: (26 commits)
  FIX: Fix bug with coreg scalars (mne-tools#12164)
  Changed casting rule in np.clip to allow reading of raw GDF files (mne-tools#12168)
  [DOC] Add documentation for setting montage order (mne-tools#12160)
  Fix inferring fiducials from EEGLAB (mne-tools#12165)
  Try to fix ICA Report (mne-tools#12167)
  BUG: Fix bug with Report.add_ica component number (mne-tools#12156)
  MAINT: Add rstcheck to CIs and pre-commit (mne-tools#12163)
  DOC: fix sphinx style typos (mne-tools#12161)
  MAINT: Fix linkcheck (mne-tools#12162)
  ENH: Add multiple label support to source_band_induced_power, source_induced_power (mne-tools#12026)
  Allow automated metadata generation to be bounded by "row events" instead of explicit time windows (mne-tools#12118)
  ENH: Collapse only in doc gen (mne-tools#12145)
  [pre-commit.ci] pre-commit autoupdate (mne-tools#12155)
  BUG: Fix bug with interior points not showing (mne-tools#12148)
  ENH: Warn about versions in sys_info (mne-tools#12146)
  Fix in conftest.py (mne-tools#12150)
  ENH: set color for bad channel with spatial_colors=True (mne-tools#12142)
  DOC: Better documentation of realign_raw (mne-tools#12135)
  Add mne-icalabel wildcard (mne-tools#12143)
  Remove LGTM.com configuration file (mne-tools#12139)
  ...
larsoner added a commit to larsoner/mne-python that referenced this pull request Nov 6, 2023
* upstream/main:
  MAINT: Add branch coverage (mne-tools#12174)
  OpenSSF (mne-tools#12175)
  fix docstring in 60_sleep.py (mne-tools#12171)
  FIX: skip empty lines in read_raw_eyelink (mne-tools#12172)
  FIX: Fix bug with coreg scalars (mne-tools#12164)
  Changed casting rule in np.clip to allow reading of raw GDF files (mne-tools#12168)
larsoner added a commit to pablomainar/mne-python that referenced this pull request Nov 8, 2023
* upstream/main:
  BUG: Fix bug with spectrum warning (mne-tools#12186)
  Add argument splash to disable splash-screen from Qt-browser (mne-tools#12185)
  BUG: Fix bug with logging and n_jobs>1 (mne-tools#12154)
  Use gray logo (works in light and dark modes) (mne-tools#12184)
  Tweak logo for dark mode (mne-tools#12176)
  ENH: Improve Covariance.__repr__ (mne-tools#12181)
  ENH: Enable sensor-specific OPM coregistration in mne coreg (mne-tools#11405)
  Tweak README.rst (mne-tools#12166)
  [pre-commit.ci] pre-commit autoupdate (mne-tools#12177)
  MAINT: Add branch coverage (mne-tools#12174)
  OpenSSF (mne-tools#12175)
  fix docstring in 60_sleep.py (mne-tools#12171)
  FIX: skip empty lines in read_raw_eyelink (mne-tools#12172)
  FIX: Fix bug with coreg scalars (mne-tools#12164)
  Changed casting rule in np.clip to allow reading of raw GDF files (mne-tools#12168)
  [DOC] Add documentation for setting montage order (mne-tools#12160)
  Fix inferring fiducials from EEGLAB (mne-tools#12165)
snwnde pushed a commit to snwnde/mne-python that referenced this pull request Mar 20, 2024
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.

Dig points disappear under transparent skin surface using mne coreg GUI

3 participants