use Image Orientation (Patient) attribute #26
Conversation
|
@carluri would you be able to take a look at this today? |
| translated_contour_data = translate_contour_to_pixel_coordinants(reshaped_contour_data, series_slice) | ||
| polygon = [np.array([translated_contour_data[:, :2]], dtype=np.int32)] | ||
| translated_contour_data = apply_transformation_to_3d_points(reshaped_contour_data, transformation_matrix) | ||
| polygon = [np.around([translated_contour_data[:, :2]]).astype(np.int32)] |
There was a problem hiding this comment.
This changes test IOUs from
0.95, 0.87, 0.85, 0.95
to
1.0, 0.96, 0.97, 1.0
There was a problem hiding this comment.
Would you mind increasing their thresholds with the tests to reflect this?
rt_utils/image_helper.py
Outdated
| @@ -22,7 +22,7 @@ def load_sorted_image_series(dicom_series_path: str): | |||
| raise Exception("No DICOM Images found in input path") | |||
|
|
|||
| # Sort slices in ascending order | |||
| series_data.sort(key=lambda ds: ds.SliceLocation, reverse=False) | |||
| series_data.sort(key=lambda ds: get_slice_position(ds), reverse=False) | |||
There was a problem hiding this comment.
SliceLocation no longer needed
tests/test_rtstruct_builder.py
Outdated
| mask[10:70, 5:15, 0] = 1 | ||
| mask[60:70, 5:40, 0] = 1 | ||
|
|
||
| IOU_threshold = 0.95 # Expected accuracy |
There was a problem hiding this comment.
You mentioned some of the test IOUs went to 1.0. Can we expect an accuracy of 1.0 here as well?
| translated_contour_data = translate_contour_to_pixel_coordinants(reshaped_contour_data, series_slice) | ||
| polygon = [np.array([translated_contour_data[:, :2]], dtype=np.int32)] | ||
| translated_contour_data = apply_transformation_to_3d_points(reshaped_contour_data, transformation_matrix) | ||
| polygon = [np.around([translated_contour_data[:, :2]]).astype(np.int32)] |
There was a problem hiding this comment.
Would you mind increasing their thresholds with the tests to reflect this?
tests/test_rtstruct_builder.py
Outdated
| @@ -164,6 +165,36 @@ def test_contour_data_sizes(new_rtstruct: RTStruct): | |||
| # Then using approximation leads to less data within the contour data | |||
| assert get_data_len_by_index(new_rtstruct, 0) < get_data_len_by_index(new_rtstruct, 1) | |||
|
|
|||
| def test_load_sorted_series(oriented_series_path): | |||
There was a problem hiding this comment.
Curious, what are we testing here?
There was a problem hiding this comment.
It was checking if we can determine ImagePosition's of all the slices using ImageOrientation and ImagePosition's of the first and last slice.
Now that I think about it this test is unnecessary, because we check the correctness of ImageOrientation inside get_slice_directions anyway. Removed it in the next commit.
rt_utils/image_helper.py
Outdated
| return mat | ||
|
|
||
| def get_patient_to_pixel_transformation_matrix(series_data): | ||
| return np.linalg.inv(get_pixel_to_patient_transformation_matrix(series_data)) |
There was a problem hiding this comment.
I've never had a chance to work with matrices in practice. Since these are just linear transformations we won't have to worry about precision errors or un-invertible matrices right?
There was a problem hiding this comment.
Yup, these are affine transformations, so they will always have an inverse.
But this line was just me being lazy. Removed it in the next commit and calculated the inverse by hand, so we don't have to worry about numerical stability of the inv function.
| return np.linalg.inv(get_pixel_to_patient_transformation_matrix(series_data)) | ||
|
|
||
| def apply_transformation_to_3d_points(points: np.ndarray, transformation_matrix: np.ndarray): | ||
| vec = np.concatenate((points, np.ones((points.shape[0], 1))), axis=1) |
There was a problem hiding this comment.
Would you mind leaving a comment explaining what is going on?
|
Awesome work @w0nsh! Really cool to see that my linear algebra courses actually came in handy. I've left a few comments and some requests for changes, only minor things really. It all looks good to me though I'm not the most familiar with matrix operations. Once my comments have been addressed, we can merge this along with #28 and create a new release 🎉 |
Needed to support DICOMs with different image orientations so I made some small modifications in image_helper.
If you'd like to merge it I can refactor the code a bit and add some tests.