NF: Wigner-D Rotation Functions

[aayush9400]

The wigner_rotation performs a Wigner rotation on a given input array 'x', representing spherical harmonics coefficients. This rotation is defined by Euler angles alpha, beta, and gamma.

Method Signature:

def wigner_rotation(signal: ndarray, alpha: int, beta: int, gamma: int) -> numpy.ndarray of same shape as image

Example Usage

import numpy as np
from keras.datasets import mnist
from utils_wigner import create_spherical, plot_spherical_img
from dipy.reconst.shm import wigner_rotation

def rotation(x, a, b, c):
    shape = x.shape
    multiples = np.append(np.ones_like(shape), shape[-1])
    x = np.tile(np.expand_dims(x, axis=-1), multiples)
    x = wigner_rotation(x, a, b, c)  
    return x[..., 0]


if __name__ == '__main__':
    (train_X, train_y), (test_X, test_y) = mnist.load_data()
    index = 0
    arr = train_X[index]    
    print("projecting {0} data image".format(train_y[index]))
    og_img = create_spherical(arr)[0]
    
    rot_img = rotation(og_img.astype(np.float32), 0, 0, 10)
    rot_img[rot_img<0] = 0
    plot_spherical_img(og_img, rot_img)

[skoudoro]

Hi @aayush9400,

Thank you for doing this! See below an initial review. I still need to go deeper in this review

[pep8speaks]

Hello @aayush9400, Thank you for updating !

• In the file dipy/reconst/shm.py:

Line 1656:69: W291 trailing whitespace
Line 1674:79: W291 trailing whitespace
Line 1683:68: W291 trailing whitespace
Line 1685:15: E741 ambiguous variable name 'l'
Line 1688:46: W291 trailing whitespace
Line 1689:15: E741 ambiguous variable name 'l'
Line 1694:75: W291 trailing whitespace
Line 1713:80: W291 trailing whitespace
Line 1715:1: W293 blank line contains whitespace
Line 1718:81: W291 trailing whitespace
Line 1729:9: E741 ambiguous variable name 'l'
Line 1740:1: W293 blank line contains whitespace

• In the file dipy/reconst/tests/test_shm.py:

Line 1042:10: W291 trailing whitespace
Line 1050:1: W293 blank line contains whitespace
Line 1052:77: W292 no newline at end of file

• In the file dipy/reconst/tests/test_wigner.py:

Line 19:5: E741 ambiguous variable name 'l'
Line 27:5: E741 ambiguous variable name 'l'
Line 36:5: E741 ambiguous variable name 'l'
Line 44:5: E741 ambiguous variable name 'l'
Line 52:5: E741 ambiguous variable name 'l'
Line 59:5: E741 ambiguous variable name 'l'
Line 77:1: W293 blank line contains whitespace
Line 80:1: W293 blank line contains whitespace
Line 82:44: W291 trailing whitespace
Line 83:1: W293 blank line contains whitespace
Line 85:34: W291 trailing whitespace

• In the file dipy/reconst/wigner.py:

Line 15:22: E741 ambiguous variable name 'l'
Line 16:76: W291 trailing whitespace
Line 24:79: W291 trailing whitespace
Line 30:79: W291 trailing whitespace
Line 31:80: W291 trailing whitespace
Line 32:77: W291 trailing whitespace
Line 37:72: W291 trailing whitespace
Line 54:33: E741 ambiguous variable name 'l'
Line 55:76: W291 trailing whitespace
Line 67:79: W291 trailing whitespace
Line 70:78: W291 trailing whitespace
Line 76:80: W291 trailing whitespace
Line 77:76: W291 trailing whitespace
Line 92:28: E741 ambiguous variable name 'l'
Line 94:77: W291 trailing whitespace
Line 95:11: W291 trailing whitespace
Line 96:1: W293 blank line contains whitespace
Line 99:43: W291 trailing whitespace
Line 104:73: W291 trailing whitespace
Line 110:77: W291 trailing whitespace
Line 136:73: W291 trailing whitespace
Line 170:12: E741 ambiguous variable name 'l'
Line 176:81: E501 line too long (81 > 80 characters)
Line 186:80: W291 trailing whitespace
Line 192:81: W291 trailing whitespace
Line 194:1: W293 blank line contains whitespace
Line 199:1: W293 blank line contains whitespace
Line 219:21: E741 ambiguous variable name 'l'
Line 220:34: W291 trailing whitespace
Line 221:45: W291 trailing whitespace
Line 222:38: W291 trailing whitespace
Line 230:79: W291 trailing whitespace
Line 233:81: W291 trailing whitespace
Line 236:74: W291 trailing whitespace
Line 240:77: W291 trailing whitespace
Line 242:76: W291 trailing whitespace
Line 247:78: W291 trailing whitespace
Line 251:80: W291 trailing whitespace
Line 256:9: E125 continuation line with same indent as next logical line
Line 276:21: E741 ambiguous variable name 'l'
Line 277:34: W291 trailing whitespace
Line 278:45: W291 trailing whitespace
Line 279:38: W291 trailing whitespace
Line 286:79: W291 trailing whitespace
Line 295:74: W291 trailing whitespace
Line 297:66: W291 trailing whitespace
Line 299:77: W291 trailing whitespace
Line 301:76: W291 trailing whitespace
Line 306:69: W291 trailing whitespace
Line 313:9: E125 continuation line with same indent as next logical line
Line 346:60: W291 trailing whitespace
Line 355:80: W291 trailing whitespace
Line 356:67: W291 trailing whitespace
Line 366:71: W291 trailing whitespace
Line 393:79: W291 trailing whitespace
Line 399:77: W291 trailing whitespace
Line 402:77: W291 trailing whitespace
Line 403:80: W291 trailing whitespace
Line 406:76: W291 trailing whitespace
Line 407:75: W291 trailing whitespace
Line 408:77: W291 trailing whitespace
Line 414:79: W291 trailing whitespace
Line 415:69: W291 trailing whitespace
Line 420:1: W293 blank line contains whitespace
Line 428:13: E741 ambiguous variable name 'l'
Line 430:49: W291 trailing whitespace
Line 431:57: W291 trailing whitespace
Line 432:50: W291 trailing whitespace
Line 450:70: W291 trailing whitespace
Line 456:74: W291 trailing whitespace
Line 457:77: W291 trailing whitespace
Line 460:79: W291 trailing whitespace
Line 463:71: W291 trailing whitespace
Line 469:72: W291 trailing whitespace
Line 470:77: W291 trailing whitespace
Line 471:57: W291 trailing whitespace
Line 510:9: E741 ambiguous variable name 'l'
Line 527:6: E114 indentation is not a multiple of four (comment)
Line 540:58: W291 trailing whitespace
Line 541:77: W291 trailing whitespace
Line 542:75: W291 trailing whitespace
Line 545:79: W291 trailing whitespace
Line 548:71: W291 trailing whitespace
Line 554:72: W291 trailing whitespace
Line 555:78: W291 trailing whitespace
Line 582:9: E741 ambiguous variable name 'l'
Line 587:43: W291 trailing whitespace
Line 588:67: W291 trailing whitespace
Line 599:71: W291 trailing whitespace
Line 608:75: W291 trailing whitespace
Line 617:80: W291 trailing whitespace
Line 627:81: W291 trailing whitespace

Comment last updated at 2024-03-10 01:07:40 UTC

[skoudoro]

Hi @aayush9400,

• Can you address all the pep8 issues?
• Can you fix the merge conflict on your branch. We can not test it for now.
• Have you look at dipy.core.geometry ? I feel like some function are duplicated/reimplemented. I need look deeper into that. I will do it after you fix the conflict. I will also look at nibabel.eulerangles. if it is ok, I would recommend to move some function in dipy.core.geometry.
• Every function should be tested individually. So you need a test_wigner.py which test every single function. it makes everything more robust and help for the review.
• See below for additional comments

Thanks for looking into that and let me know when I can do a deeper review and test it.

[skoudoro]

I understand. That's why I propose you to increase the dimension.

from [..., beta, alpha, gamma], create new array with [..., beta, alpha, gamma, index] where instead of list of ndarray which has memory issues.

[skoudoro]

I do not think this function should be in this module.

[aayush9400]

should i put it in dipy.core.geometry?

[Garyfallidis]

Okay. Good progress. Next step is adding simple to follow examples.