Skip to content

Conversation

@drammock
Copy link
Member

This should fix the mesh display color bug that has been in tutorials/forward/50_background_freesurfer_mne.py since the 1.0 release (last known good was 0.24). Unfortunately we probably won't be able to tell because that tutorial is bricking Circle in other ways right now:

generating gallery for auto_tutorials/forward... [ 75%] 50_background_freesurfer_mne.py
Clean 1636.6 s : 1.73 GB
qt.qpa.xcb: QXcbConnection: XCB error: 128 (Unknown), sequence: 15178, resource id: 2098234, major code: 130 (Unknown), minor code: 2

...and I haven't been able to pinpoint the cause of that yet.

@larsoner larsoner added this to the 1.2 milestone Sep 26, 2022
@drammock
Copy link
Member Author

hmm... macOS CIs do not actually fail here. Only failure is this on Windows 3.9 conda, in test test_get_coef_multiclass_full[4-10-2] and it's a ThreadExceptionWarning (unrelated?):

https://dev.azure.com/mne-tools/mne-python/_build/results?buildId=22143&view=logs&j=2bd7b19d-6351-5e7f-8417-63f327ab45bc&t=0c49e006-47ab-5675-a040-5d301cde7356&l=3354

But unfortunately the rendered tutorial is better, but not all the way fixed:

https://output.circle-artifacts.com/output/job/ea11ac33-7953-417e-ad83-aefc14a19f85/artifacts/0/dev/auto_tutorials/forward/50_background_freesurfer_mne.html#cortical-alignment-spherical

Locally it looks correct for me though. More debugging necessary.

* upstream/main:
  MAINT: Fix CircleCI error (mne-tools#11205) [circle deploy]
  Add regression-based approach to removing EOG artifacts (mne-tools#11046)
  [DOC, MRG] Minor documentation improvements and remove glossary entry for array-like (mne-tools#11207)
  Fix `include_tmax` not considered in `mne.io.Raw.crop` to check `tmax` in bounds (mne-tools#11204)
  MAINT: Fix notebook backend (mne-tools#11206)
  MRG: Fix displayed Raw duration in Jupyter notebook (mne-tools#11203)
  add EpochsSpectrum.average() (mne-tools#11198)
@larsoner larsoner marked this pull request as ready for review September 28, 2022 06:42
@larsoner
Copy link
Member

Locally it looks correct for me though. More debugging necessary.

My initial reaction to this was that it was probably a MESA bug. Locally I got good plots natively, but could replicate with xvfb-run, which uses MESA under the hood:

$ MNE_3D_OPTION_MULTI_SAMPLES=1 MNE_SKIP_INSTANCE_ASSERTIONS=1 PATTERN=50_background_freesurfer xvfb-run make html_dev-pattern-memory

Once I had that, I could debug locally to figure it out. Things I changed:

  1. Deal with what must be a MESA wireframe bug by setting line_width > 1 and render_lines_as_tubes=True. It changes the rendering sufficiently that it renders properly.
  2. Automatically set smooth_shading=False when representation='wireframe' inside renderer.mesh() because you never want it on for a wireframe (it won't render properly I don't think)
  3. Cull to the +X tris so that you only get one set in the view (which is from the right), and change the distance to 150 (the spheres extend to 100 so this is a nice distance).
  4. Fix a camera reset bug -- it wasn't affecting the plots but was a bug that I noticed. We shouldn't use the default render=True in our reset_camera call within set_view but rather call plotter.reset_camera(render=False) because we then immediately change some stuff about the view, then (optionally) call update. This should fix some jerky Brain behavior I've never been able to track down until now where opening a window causes the views to "flash" a bit on startup. It should also make things slightly faster because it avoids a render of a view that is never used.

You can see the fixed outputs here:

https://output.circle-artifacts.com/output/job/c42d266b-d50e-461c-a41c-f6cf7f8951f1/artifacts/0/dev/auto_tutorials/forward/50_background_freesurfer_mne.html#cortical-alignment-spherical

image

image

image

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.

🙏

@agramfort agramfort enabled auto-merge (squash) September 28, 2022 06:56
@larsoner larsoner disabled auto-merge September 28, 2022 15:12
@larsoner larsoner merged commit b6e4f5c into mne-tools:main Sep 28, 2022
larsoner added a commit to drammock/mne-python that referenced this pull request Sep 28, 2022
…-args

* upstream/main:
  Fix mesh display in tutorial (mne-tools#11200)
  MAINT: Add arm64 CI using CirrusCI (mne-tools#11209)
  Fix spatial colors (mne-tools#11201)
  MAINT: Fix CircleCI error (mne-tools#11205) [circle deploy]
  Add regression-based approach to removing EOG artifacts (mne-tools#11046)
  [DOC, MRG] Minor documentation improvements and remove glossary entry for array-like (mne-tools#11207)
  Fix `include_tmax` not considered in `mne.io.Raw.crop` to check `tmax` in bounds (mne-tools#11204)
  MAINT: Fix notebook backend (mne-tools#11206)
  MRG: Fix displayed Raw duration in Jupyter notebook (mne-tools#11203)
  add EpochsSpectrum.average() (mne-tools#11198)
@drammock drammock deleted the fix-mesh-display branch September 28, 2022 15:21
larsoner added a commit to larsoner/mne-python that referenced this pull request Oct 11, 2022
* upstream/main: (64 commits)
  MAINT: Better check (mne-tools#11229)
  MAINT: Fix link and update instantiation note (mne-tools#11228)
  BUG: Add estimated fiducials when missing / assumed head coords (mne-tools#11212)
  Fix tfr db (mne-tools#11223)
  MAINT: Update link (mne-tools#11222)
  add CPGRL doc section (mne-tools#11216)
  Don't insert superfluous newlines in subprocess log messages (mne-tools#11219)
  purge _get_args helper func (mne-tools#11215)
  Standardize topomap args (mne-tools#11123)
  MAINT: Ensure no datasets are downloaded in tests (mne-tools#11213)
  MAINT: Fix Cirrus caching (mne-tools#11211)
  Fix mesh display in tutorial (mne-tools#11200)
  MAINT: Add arm64 CI using CirrusCI (mne-tools#11209)
  Fix spatial colors (mne-tools#11201)
  MAINT: Fix CircleCI error (mne-tools#11205) [circle deploy]
  Add regression-based approach to removing EOG artifacts (mne-tools#11046)
  [DOC, MRG] Minor documentation improvements and remove glossary entry for array-like (mne-tools#11207)
  Fix `include_tmax` not considered in `mne.io.Raw.crop` to check `tmax` in bounds (mne-tools#11204)
  MAINT: Fix notebook backend (mne-tools#11206)
  MRG: Fix displayed Raw duration in Jupyter notebook (mne-tools#11203)
  ...
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.

3 participants