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 gaussian process DWI signal representation notebook #202

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

jhlegarreta
Copy link
Collaborator

Add gaussian process DWI signal representation notebook.

@jhlegarreta
Copy link
Collaborator Author

Depends on #188. @esavary If you did changes to the notebook, maybe you can do a PR to this branch with such changes, and I would merge it right away.

With the default kernels, I get this error plot, so the default kernels are not a good choice for the DWI signal:

gaussian_process_default_kernel

Not sure if the signal not being normalized plays a role here.

@esavary
Copy link
Member

esavary commented Jun 5, 2024

I made a pull request against your branch with some suggestions. Essentially, it involves not fitting one voxel at a time. This ensures that the same hyperparameters are used for the kernel across all voxels. It also improves their estimation when done with more voxels (this is done inside the fit method). These changes result in a slight improvement. However, even with these changes, the optimization is still failing for the parameter k2__noise_level (from the WhiteNoise kernel). It hits the bounds, leading to a significant error, as shown in this plot (I used the default hyperparameters optimization from the fit method of sklearn not the custom one I coded):

results_small_cube

Additionally, I tried fitting a larger cube, but it seems to be similar as with the SphericalKernel. See the image below showing the prediction:

estimationwithekernel

An additional comment I have is that we might consider using the angles between the directions instead of bvecs directly, as suggested in Andersson (2015). However, I was unsure if that was only relevant when using the Spherical or Exponential kernel, so I did not include that change.

@esavary
Copy link
Member

esavary commented Jun 5, 2024

Depends on #188. @esavary If you did changes to the notebook, maybe you can do a PR to this branch with such changes, and I would merge it right away.

With the default kernels, I get this error plot, so the default kernels are not a good choice for the DWI signal:

gaussian_process_default_kernel

Not sure if the signal not being normalized plays a role here.

Actually, I've just noticed something in your plot, I think the ground truth signal should not be zero (but not sure if it causes the problem). Could it be that the data you used for the test is empty except for the first gradient direction? ( I used this data for my plot: https://osf.io/download/6at98/)

@jhlegarreta
Copy link
Collaborator Author

Merged your PR into the branch.

Maybe we can now uncomment the orthoview calls and provide with the same values you used to generate the figures you pasted.

An additional comment I have is that we might consider using the angles between the directions instead of bvecs directly, as suggested in Andersson (2015). However, I was unsure if that was only relevant when using the Spherical or Exponential kernel, so I did not include that change.

In my understanding we want to predict on bvecs. We want to represent a given DWI volume using a GP, e.g. as if we were using another basis (e.g. SH) to represent it and be able to recreate it within some tolerance. The ISMRM paper/poster figure is helpful in this.

Actually, I've just noticed something in your plot, I think the ground truth signal should not be zero (but not sure if it causes the problem). Could it be that the data you used for the test is empty except for the first gradient direction? ( I used this data for my plot: https://osf.io/download/6at98/)

Maybe the volume I chose included some background voxels.

@jhlegarreta
Copy link
Collaborator Author

Maybe we should normalize the DWI signal (divide each voxel signal by the B0 signal in that location) prior to fitting the data. Not sure if this would have a large influence on the results, but maybe worthwhile trying.

@jhlegarreta
Copy link
Collaborator Author

Added a new notebook with a simulated signal using a multi-tensor model.

@arokem In an effort to get something like fig 4 in Andersson 2015 NIMG, I am getting the below figure for the signal I am simulating in docs/notebooks/dwi_simulated_gp.ipynb.

dwi_multi-tensor_signal_90dirs

I would expects the diffusion signals to roughly describe the two crossing tensors, but what I get is a blob. Tensors are at 90 degrees, and the fODF I get from them shows so:

dwi_multi-tensor_signal_90dirs_fodf

What am I missing? Thanks.

@arokem
Copy link
Collaborator

arokem commented Jul 1, 2024

Sorry for the slowness. One quick thought is that Andersson et al. used a b=3,000 in their simulations and I see that you are using b=1,000. Another is that I suspect that the Andersson figures were generated with a transformation to diffusion distance, rather than in the original signal domain. Indeed, a crossing fiber signal would look like the linear combination of two "donuts" rather than the linear combination of two "cigars", because the signal looks like a donut (see e.g., figure 1 in my 2014 paper), and when you add these donuts together, especially in low b-values, you get something that more closely resembles a "blob". To get "cigars" you need to do something like this: https://github.com/arokem/osmosis/blob/72d848ddc172ce6a72baba4c0d4cf3675db1a381/osmosis/tensor.py#L233-L248

@jhlegarreta
Copy link
Collaborator Author

Removed the signal plot (#202 (comment)) for now so that it does not distract us as we progress with the kernels, etc. Will add it in a separate PR on top of this branch.

@oesteban
Copy link
Member

@jhlegarreta can you commit with the cells' output so that we can use github to visualize the results? It's tough to look at the diff, but we workaround the problem of the HCP data.

@jhlegarreta
Copy link
Collaborator Author

jhlegarreta commented Jul 10, 2024

@jhlegarreta can you commit with the cells' output so that we can use github to visualize the results? It's tough to look at the diff, but we workaround the problem of the HCP data.

Brought back the cell in the last push force:
https://github.com/nipreps/eddymotion/blob/40baac4d3225060bc83ab5daf95a026c10f115c9/docs/notebooks/dwi_simulated_gp.ipynb

Note that in PR #217 I had tried with the approach Ariel suggested but did not get the expected shape.

Add gaussian process DWI signal representation notebooks:
- One of the notebooks uses a simulated DWI signal.
- The second notebook uses a real DWI signal.
Add singla plot cells.
"# Create an interactive plot\n",
"%matplotlib notebook\n",
"\n",
"fig = plot_dwi_scatter(signal_repr)\n",
Copy link
Collaborator Author

@jhlegarreta jhlegarreta Jul 12, 2024

Choose a reason for hiding this comment

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

Shows two disks, so not quite right yet.

signal_scatter_2

signal_scatter_1

Installed ipympl to be able to interact with (rotate) the mpl figures.

"cell_type": "code",
"source": [
"# ToDo\n",
"# Plot as surface"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Have not succeeded in getting this right.

"ax.set_xlabel(\"bvec indices\")\n",
"ax.set_ylabel(\"signal\")\n",
"ax.legend()\n",
"plt.title(\"Gaussian process regression on dataset\")\n",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The plot looks perfect because I am predicting on the very same data that I used for fitting.

@jhlegarreta jhlegarreta force-pushed the AddDWIGPNotebook branch 2 times, most recently from bdbdffd to 5d17dca Compare August 11, 2024 22:36
Experiment using a single tensor in the GP notebook.
@jhlegarreta
Copy link
Collaborator Author

jhlegarreta commented Aug 11, 2024

A few notes:

  • Have not investigated the optimization convergence error shown on the notebok (ABNORMAL_TERMINATION_IN_LNSRCH.): when training and predicting on all bvecs (except b0) the warning was being shown, but not the error.
  • I think I have figured out how to properly create the surface plots of the diffusion signal (which could be used for the prediction) and have it in a separate piece of code. When we take the time to discuss about how confident we are about the GP (e.g. the differences wrt the previous implementation, etc.), and other issues (example organisation, data and code to generate the empirical covariance plot) we can bring that part.

@oesteban
Copy link
Member

Is there anything I can do here before the TechMon meeting?

@jhlegarreta
Copy link
Collaborator Author

@oesteban I apologise for not having worked on this in the last few weeks; I expect to be able to have another look around the tech meeting date. Sorry.

The single point prediction looks to be working, but I am not sure about the source of the error message mentioned in #202 (comment), so I need to investigate that. I would like to pick other random b-vecs to see if the prediction is OK or whether the convergence message persists.

Additionally, I would like to investigate what has changed wrt the implementation that was not working. I need to prepare
multi-fiber settings, and add the predicted point visualization part. So all this is on my roof; if you feel like, you can run the notebook and see if my impression that it is working seems OK (despite the convergence message).

Probably we can talk about all these experiments/design (esp. when we will add experiments with acquired data) in our eventual meeting.

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.

None yet

4 participants