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

ENH: Add gradient plot method #1292

Closed

Conversation

jhlegarreta
Copy link
Contributor

Add gradient plot method.

Add gradient plot method.
@jhlegarreta
Copy link
Contributor Author

Notes:

  • Or do we prefer each shell in a different sphere, e.g.?

@oesteban
Copy link
Member

I would prefer the first plot, but with a twist: it should render both the original b-vectors and where they land after head motion correction, possibly showing the glyph through which they rotate.

The original b-vector could be just a point (not to overcrowd the plot) and then the rotated b-vector should take the color scale.

Why this?

First, because the plot would allow assessing head motion at the same time (we have some metrics summarizing the angles between original and rotated b-vectors).
Second, because MRIQC cannot assume the dataset is "shelled" -- the input can be DSI or some hybrid sampling. IMHO, plots with 3 or more spheres are almost impossible to interpret. Having a single sphere can feel crowdy, but gives a good sense of the coverage of the b-vectors and shells.

WDYT?

@oesteban
Copy link
Member

  • This has been transferred/adapted (return object docstring added to plot_gradients) from eddymotion. IMO eddymotion should import plot capabilities from either here, nireports or elsewhere. As the mentioned issue was opened here, adding the code here.

I think eddymotion's viz module could be outsourced into nireports, so all nipreps can use it naturally without forcing the dependency.

@oesteban
Copy link
Member

  • Not sure whether the module where I added the data is the most appropriate one (e.g. instrumentation has a viz module).

mriqc.instrumentation is basically a resource monitoring tool, and the plotting is just for it. mriqc.viz is the right location for this (actually somewhere under nireports.reportlets)

Copy link
Member

@oesteban oesteban left a comment

Choose a reason for hiding this comment

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

After quickly looking to the code: should we directly outsource this from eddymotion into nireports?

@jhlegarreta
Copy link
Contributor Author

I would prefer the first plot, but with a twist: it should render both the original b-vectors and where they land after head motion correction, possibly showing the glyph through which they rotate.
First, because the plot would allow assessing head motion at the same time (we have some metrics summarizing the angles between original and rotated b-vectors).

Agreed. But I would leave that for a separate PR. See below.

because MRIQC cannot assume the dataset is "shelled" -- the input can be DSI or some hybrid sampling. IMHO, plots with 3 or more spheres are almost impossible to interpret. Having a single sphere can feel crowdy, but gives a good sense of the coverage of the b-vectors and shells.

OK, I see. For now, I would prefer to go forward with the code as I extracted from eddymotion: it is already a useful tool. Identifying the relevant data (conventional shelled DWI data or DSI acq data) is already missing, and I fear that it may take some time until everything is in place. Same goes to the corrected bvecs.

After quickly looking to the code: should we directly outsource this from eddymotion into nireports?

OK. So I will close this PR and move the code nireports.

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.

2 participants