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.

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

[BUG] avg_method and symmetric_cbar flags using plot_surf_stat_map working properly with plotly engine #4296

Open
3 of 9 tasks
albertofpena opened this issue Feb 28, 2024 · 16 comments
Labels
Bug for bug reports Plotting The issue is related to plotting functionalities.

Comments

@albertofpena
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

Operating system

  • Linux
  • Mac
  • Windows

Operating system version

Debian testing (trixie)

Python version

  • 3.12
  • 3.11
  • 3.10
  • 3.9
  • 3.8

nilearn version

0.10.3

Expected behavior

When using the plotly engine, the colorbar should not be symmetric if symmetric_cbar=False. The avg_method should be user settable. This seems related to #3941 bug.

Current behavior & error messages

This is what I got:

The flag symmetric_cbar=False is ignored, always plotting symmetric colorbars. On the other hand, when avg_method flag is inserted, I get the following error:

---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
Cell In[7], line 3
      1 from nilearn import plotting
----> 3 fig = plotting.plot_surf_stat_map(
      4     surf, texture, hemi='left',
      5     title='Surface left hemisphere', colorbar=True,
      6     engine='plotly', symmetric_cbar=False, avg_method='mean',
      7     darkness=0.5, cmap='black_red'
      8 )
     10 # change plotly lighting
     11 fig.figure.data[0].lighting.ambient = 0.5

File ~/.local/lib/python3.11/site-packages/nilearn/plotting/surf_plotting.py:1151, in plot_surf_stat_map(surf_mesh, stat_map, bg_map, hemi, view, engine, threshold, alpha, vmin, vmax, cmap, colorbar, symmetric_cbar, cbar_tick_format, bg_on_data, darkness, title, title_font_size, output_file, axes, figure, **kwargs)
   1142 # Call get_colorbar_and_data_ranges to derive symmetric vmin, vmax
   1143 # And colorbar limits depending on symmetric_cbar settings
   1144 cbar_vmin, cbar_vmax, vmin, vmax = get_colorbar_and_data_ranges(
   1145     loaded_stat_map,
   1146     vmin=vmin,
   1147     vmax=vmax,
   1148     symmetric_cbar=symmetric_cbar,
   1149 )
-> 1151 display = plot_surf(
   1152     surf_mesh, surf_map=loaded_stat_map,
   1153     bg_map=bg_map, hemi=hemi,
   1154     view=view, engine=engine, avg_method='mean', threshold=threshold,
   1155     cmap=cmap, symmetric_cmap=True, colorbar=colorbar,
   1156     cbar_tick_format=cbar_tick_format, alpha=alpha,
   1157     bg_on_data=bg_on_data, darkness=darkness,
   1158     vmax=vmax, vmin=vmin,
   1159     title=title, title_font_size=title_font_size, output_file=output_file,
   1160     axes=axes, figure=figure, cbar_vmin=cbar_vmin,
   1161     cbar_vmax=cbar_vmax, **kwargs
   1162 )
   1163 return display

TypeError: nilearn.plotting.surf_plotting.plot_surf() got multiple values for keyword argument 'avg_method'

Steps and code to reproduce bug

stat_img = image.smooth_img("TFCE_log_pDR_0002.nii", fwhm="fast")
surf = surface.load_surf_mesh('~/Downloads/Surf_Ice/BrainNet/BrainMesh_ICBM152_smoothed.lh.gii')
texture = surface.vol_to_surf(stat_img, surf, radius=3, kind='ball')

fig = plotting.plot_surf_stat_map(
    surf, texture, hemi='left',
    title='Surface left hemisphere', colorbar=True,
    engine='plotly', symmetric_cbar=False, avg_method='mean',
    darkness=0.5
)

# change plotly lighting
fig.figure.data[0].lighting.ambient = 0.5
fig.figure.data[0].lighting.diffuse = 0.6
fig.figure.data[0].lighting.specular = 0.1
fig.figure.data[0].lighting.roughness = 0.03
fig.figure.data[0].lighting.fresnel = 0.05
fig.figure.data[0].lightposition = dict(x=0, y=20, z=30)

fig.show()
@albertofpena albertofpena added the Bug for bug reports label Feb 28, 2024
@albertofpena albertofpena changed the title [BUG] avg_method and symmetric_cbar in plotly not working [BUG] avg_method and symmetric_cbar flags in plotly not working properly Feb 28, 2024
@albertofpena albertofpena changed the title [BUG] avg_method and symmetric_cbar flags in plotly not working properly [BUG] avg_method and symmetric_cbar flags using plot_surf_stat_map with plotly not working properly Feb 28, 2024
@albertofpena albertofpena changed the title [BUG] avg_method and symmetric_cbar flags using plot_surf_stat_map with plotly not working properly [BUG] avg_method and symmetric_cbar flags using plot_surf_stat_map working properly with plotly engine Feb 28, 2024
@bthirion
Copy link
Member

Thx for reporting. Could you make it work with images provided or generated by Nilearn ? This would make it easier for us.

@Remi-Gau
Copy link
Collaborator

For the avg_method, the error you are getting is because plot_surf_stat_map will call plot_surf with avg_method='mean' so you cannot at the moment pass it another value (or even the same).

view=view, engine=engine, avg_method='mean', threshold=threshold,

Also note that avg_method is not implemented for plotly:

https://nilearn.github.io/stable/modules/generated/nilearn.plotting.plot_surf.html#nilearn.plotting.plot_surf

image

@Remi-Gau
Copy link
Collaborator

Went down the git blame 🐰 hole and it seems that this avg_method='mean' has been there for a long time.

This is the PR that set it I think.
#1387

@bthirion: any reason why we should force users to only plot the mean for a surface?

@Remi-Gau
Copy link
Collaborator

quick check and removing that avg_method='mean' for plot_surf_stat_map and trying to pass min or max makes no difference from what I can tell... Which is... surprising...

@Remi-Gau
Copy link
Collaborator

I can reproduce the symmetric_bar issue with plotly with this code.

import numpy as np

from nilearn import datasets, plotting, surface

import matplotlib.pyplot as plt

stat_img = datasets.load_sample_motor_activation_image()

fsaverage = datasets.fetch_surf_fsaverage()

curv_right = surface.load_surf_data(fsaverage.curv_right)
curv_right_sign = np.sign(curv_right)

texture = surface.vol_to_surf(stat_img, fsaverage.pial_right)

engine = "matplotlib"

plotting.plot_surf_stat_map(
    fsaverage.infl_right,
    texture,
    hemi="right",
    title="Surface right hemisphere",
    colorbar=True,
    threshold=1.0,
    bg_map=curv_right_sign,
    engine=engine,
    symmetric_cbar=False,
)
plt.show()

engine = "plotly"

print(f"Using plotting engine {engine}.")

fig = plotting.plot_surf_stat_map(
    fsaverage.infl_right,
    texture,
    hemi="right",
    title="Surface right hemisphere",
    colorbar=True,
    threshold=1.0,
    bg_map=curv_right_sign,
    bg_on_data=True,
    engine=engine,
    symmetric_cbar=False,
)
fig.show()

@Remi-Gau
Copy link
Collaborator

Will open a draft PR to discuss this...

@albertofpena
Copy link
Author

albertofpena commented Feb 29, 2024

Thanks for your quick response! Please let me know if I can help in any way.

@bthirion
Copy link
Member

Went down the git blame 🐰 hole and it seems that this avg_method='mean' has been there for a long time.

This is the PR that set it I think. #1387

@bthirion: any reason why we should force users to only plot the mean for a surface?

AFAIK there is no technical reason.

@Remi-Gau
Copy link
Collaborator

Remi-Gau commented Mar 6, 2024

quick check and removing that avg_method='mean' for plot_surf_stat_map and trying to pass min or max makes no difference from what I can tell... Which is... surprising...

probably explained by the fact that a statistical map is a 3D volume to there is only one value along the 4th dimension, so min, max, mean will always be the same.

@albertofpena
Copy link
Author

quick check and removing that avg_method='mean' for plot_surf_stat_map and trying to pass min or max makes no difference from what I can tell... Which is... surprising...

probably explained by the fact that a statistical map is a 3D volume to there is only one value along the 4th dimension, so min, max, mean will always be the same.

Probably it's my mistake, but I assumed that the average was done at a vertex/face level. Also, in this case you pass the statistical map as a texture, not as a 3D volume per se, so I'm not sure how it would affect.

About the colorbar issue, I tested the fixes in your pull request and it seems to do the trick. Thanks for your help.

@Remi-Gau
Copy link
Collaborator

Remi-Gau commented Mar 7, 2024

Probably it's my mistake, but I assumed that the average was done at a vertex/face level.

Gosh... You are completely correct...

@Remi-Gau
Copy link
Collaborator

Remi-Gau commented Mar 7, 2024

Also, in this case you pass the statistical map as a texture, not as a 3D volume per se, so I'm not sure how it would affect.

I was trying to stay close to the code you provided but this did lead me to try passing the path to a nifti image as stat map as stat_map as the doc string says it should be ok but this does not seem to work:

image

import matplotlib.pyplot as plt
import numpy as np

from nilearn import datasets, plotting, surface

fsaverage = datasets.fetch_surf_fsaverage()
curv_left = surface.load_surf_data(fsaverage.curv_left)
curv_left_sign = np.sign(curv_left)

stat_map = datasets.load_sample_motor_activation_image()

print(stat_map)

# %%
engine = "matplotlib"

plotting.plot_surf_stat_map(
    fsaverage.infl_left,
    stat_map = stat_map,
    hemi="left",
    title="Surface left hemisphere",
    cmap = 'bwr',
    colorbar=True,
    threshold=1.0,
    bg_map=curv_left_sign,
    engine=engine,
    symmetric_cbar=False,
    avg_method="mean",
)

gives

/home/remi/github/nilearn/nilearn/nilearn/datasets/data/image_10426.nii.gz

Traceback (most recent call last):
  File "/home/remi/github/nilearn/nilearn/tmp.py", line 22, in <module>
    plotting.plot_surf_stat_map(
  File "/home/remi/github/nilearn/nilearn/nilearn/plotting/surf_plotting.py", line 1167, in plot_surf_stat_map
    display = plot_surf(
              ^^^^^^^^^^
  File "/home/remi/github/nilearn/nilearn/nilearn/plotting/surf_plotting.py", line 809, in plot_surf
    fig = _plot_surf_matplotlib(
          ^^^^^^^^^^^^^^^^^^^^^^
  File "/home/remi/github/nilearn/nilearn/nilearn/plotting/surf_plotting.py", line 580, in _plot_surf_matplotlib
    surf_map_faces = _compute_surf_map_faces_matplotlib(
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/remi/github/nilearn/nilearn/nilearn/plotting/surf_plotting.py", line 366, in _compute_surf_map_faces_matplotlib
    surf_map_data = _check_surf_map(surf_map, n_vertices)
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/remi/github/nilearn/nilearn/nilearn/plotting/surf_plotting.py", line 345, in _check_surf_map
    raise ValueError("'surf_map' can only have one dimension "
ValueError: 'surf_map' can only have one dimension but has '3' dimensions

@Remi-Gau
Copy link
Collaborator

Remi-Gau commented Mar 7, 2024

OK so I will split this into several PR to solve because there is more than one issue:

  • dealing with avg_method
  • dealing with symmetric_cbar
  • and this thing that nifti cannot be passed to surface plotting even though the doc says so.

@Remi-Gau Remi-Gau added this to the Release 0.10.4 milestone Mar 18, 2024
@Remi-Gau Remi-Gau added the Plotting The issue is related to plotting functionalities. label Mar 19, 2024
@Remi-Gau
Copy link
Collaborator

Remi-Gau commented Mar 22, 2024

For the symmetric_cbar with plot_surf_stat_map.

Just writing downs the things to check.

@Remi-Gau
Copy link
Collaborator

Note passing a negative values as threshold is ignored and does not throw warnings:

"""Test plotting asymmetric color bar."""

import matplotlib.pyplot as plt
import numpy as np

from nilearn import datasets, plotting, surface
from nilearn.image import threshold_img

vmin = None
vmax = None
symmetric_cbar = False
threshold = -0.5 

stat_img = datasets.load_sample_motor_activation_image()

fsaverage = datasets.fetch_surf_fsaverage()

curv_right = surface.load_surf_data(fsaverage.curv_right)
curv_right_sign = np.sign(curv_right)

texture = surface.vol_to_surf(stat_img, fsaverage.pial_right)

engine = "matplotlib"

plotting.plot_surf_stat_map(
    fsaverage.infl_right,
    texture,
    hemi="right",
    title="Surface right hemisphere",
    colorbar=True,
    threshold=threshold,
    bg_map=curv_right_sign,
    engine=engine,
    symmetric_cbar=False,
    cmap="black_red",
    vmin = vmin,
    vmax = vmax
)
plt.show()

gives

Figure_1

Where as I would expect the same as for threshold = 0.5 given that the threshold is supposed to act on absolute value according to the doc.

Figure_2

@Remi-Gau
Copy link
Collaborator

Tried passing invalid values to symmetric_cbar and I not getting errors. Will add some input validation.

@Remi-Gau Remi-Gau added this to the Release 0.11.0 milestone Apr 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug for bug reports Plotting The issue is related to plotting functionalities.
Projects
None yet
Development

No branches or pull requests

3 participants