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: wrong origin location in orthoview, update OrthoSlicer3D._set_position in viewers.py #1319

Merged
merged 4 commits into from
Sep 5, 2024

Conversation

gjpcbecq
Copy link
Contributor

correct a bug with image.orthoview() from class nibabel.viewers.OrthoSlicer3D

There is a wrong selection of indices from original data using the inverse affine transform, leading to weird selection of voxels

This bug is also related to strange behavior with special acquisition, for example with small animal settings such as rodents where PA and IS axis are transposed (LSA system), leading to wrong location of origin (0,0,0) with image.orthoview()

For example, this small example will generate wrong localization of the referential with the main code and will result in good representation with the proposed correction:

data1 = np.random.rand(20, 20, 40)
data1[10, 10, :] = 0
data1[10, :, 30] = 0
data1[:, 10, 30] = 0
aff1 = np.array([[1, 0, 0, -10], [0, 0, 1, -30], [0, 1, 0, -10], [0, 0, 0, 1]])
nib.viewers.OrthoSlicer3D(data1, aff1)

@gjpcbecq gjpcbecq changed the title Update OrthoSlicer3D._set_position in viewers.py BUG: wrong origin location in orthoview, update OrthoSlicer3D._set_position in viewers.py Apr 29, 2024
wrong indices to original data leading to weird selection of voxels for weird affine transforms and weird volumes

this bug is also related to strange behavior with special acquisition, for example with small animal settings such as rodents leading to wrong location of origin (0,0,0) with image.orthoview()
@effigies
Copy link
Member

Thanks for this! Would it be possible to get a test that clearly demonstrates the error before and is fixed after?

Copy link

codecov bot commented Apr 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.35%. Comparing base (d15ec58) to head (cdc788d).
Report is 7 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1319   +/-   ##
=======================================
  Coverage   95.34%   95.35%           
=======================================
  Files         207      207           
  Lines       29481    29504   +23     
  Branches     4981     4982    +1     
=======================================
+ Hits        28110    28133   +23     
  Misses        930      930           
  Partials      441      441           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@gjpcbecq
Copy link
Contributor Author

Thanks for this! Would it be possible to get a test that clearly demonstrates the error before and is fixed after?

Hi Chris,
You can try this code in a module for example test_ortho.py:

import matplotlib
matplotlib.use('Qt5Agg') # or any functional backend. 
import matplotlib.pyplot as plt
import nibabel as nib
import numpy as np
data1 = np.random.rand(10, 20, 40)
data1[5, 10, :] = 0
data1[5, :, 30] = 0
data1[:, 10, 30] = 0
aff1 = np.array([[1, 0, 0, -5], [0, 0, 1, -30], [0, 1, 0, -10], [0, 0, 0, 1]])
o1 = nib.viewers.OrthoSlicer3D(data1, aff1)
o1.show()

This creates a 3D array containing random values, with more elements in the third dimension than in the others dimensions.
Values for the axes of the frame of reference and origin at (5, 30, 10) are set to 0.
The affine transform to the RAS system associates LR with dim 0, PA to dim 2 and IS to dim 1.

$$\mbox{Aff}_1 = \left(\begin{array}{cccc} 1 & 0 & 0 & -5 \\\ 0 & 0 & 1 & -30 \\\ 0 & 1 & 0 & -10 \\\ 0 & 0 & 0 & 1 \end{array}\right)$$

using OrthoSlicer3D the referential in green should override the black lines in the volume.
The visual result of the use of this code is given in Fig.1 :
a) with the initial version of the code, the green lines are not aligned with the black lines in the volume.
b) with the proposed correction, the green lines are aligned with the black lines in the volume.

Fig.1

Fig.1

It seems to have other bugs in the OrthoSlicer3D that I try to fix. This will be proposed in other pull requests.

@effigies
Copy link
Member

effigies commented Sep 5, 2024

Hi @gjpcbecq! I'm going to try to turn your MRE into a test.

Apologies for taking so long to get back to this. I'm commenting here to try to pressure myself to spend some time on it. :-)

@effigies
Copy link
Member

effigies commented Sep 5, 2024

Okay, so I've had a go at reproducing this, and your fix resolves the initial placement at the origin, but if you attempt to click around to other locations with the mouse, you no longer go the place you click. Removing your patch makes the mouse work.

So I think there's something more fundamentally wrong here. It's possible that we just need to do the inverse of this in the _on_mouse() function... I'm going to spend a bit more time on this, but if you have a chance to look at it, feel free to take a stab.

@effigies effigies merged commit abe765f into nipy:master Sep 5, 2024
48 of 49 checks passed
@gjpcbecq gjpcbecq deleted the correct-bug-OrthoSlicer3D branch September 6, 2024 07:27
@gjpcbecq
Copy link
Contributor Author

gjpcbecq commented Sep 6, 2024

Thank you Chris for working on this.
I was working how to check for the data in the axes for the test, but your solution is very nice.
There is yet the problem with the _on_mouse function.
I am working on it and I will propose you next week a solution in a new pull request with a test on the test_viewers.py to check it.
Best regards,
Guillaume

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