-
Notifications
You must be signed in to change notification settings - Fork 12
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
Updated ghosting function – fixes sampling point issue #185
Conversation
Hello @laurencejackson, @tomaroberts , I have fixed the ghosting script. The biggest problem was that the ghosting slice was always placed at random points in the image, not where it should have been, namely int the phase encoding direction. There is now a new function called get_ghosting_slice2 which selects the correct ghosting slice. I removed the old get_ghosting_slice function. I have had to change the code slightly in the main function and in the report_path if statement because it didn't plot correctly. Finally I changed the rescale_to_byte function because the original one changes the values and spreads them into a more widened histogram. I can put this back how it was but I think ideally that function will need to be changed at some point for all the hazenlib scripts.
I updated the test_ghosting script to work with the new ghosting function. Now the get_ghost_slice needs to be equal to the eligible area.
Coverage Report
|
Hi @laurencejackson , @tomaroberts I fixed the ghosting script. I also fixed the tests. |
I have also fixed the issue which said: prevent ghosting from outputting negative values. That issue is linked. For that I only had to add an absolute value in the ghosting formula. @tomaroberts @laurencejackson |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for all this Lu – see review comments. Main ones are that the code is always outputting True.png
and it would be good to get a bit more clarity on the difference between rescale_to_byte
and your version. Thanks!
@@ -223,25 +244,28 @@ def get_ghosting(dcm, plotting=False) -> dict: | |||
img = cv.rectangle(img.copy(), (x1, y1), (x2, y2), (255, 0, 0), 1) | |||
|
|||
ax.imshow(img) | |||
fig.savefig(f'{report_path}.png') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I run hazen ghosting ... --report
it outputs a file called True.png
. I think it's because you have used report_path
here, which = True or False.
On a related note, the function always outputs True.png
, even without the --report
option. Is that supposed to happen?
signal_centre = [(bbox[0]+bbox[1])//2, (bbox[2]+bbox[3])//2] | ||
background_rois = get_background_rois(dcm, signal_centre) | ||
ghost_col, ghost_row = get_ghost_slice(bbox, dcm, slice_radius=slice_radius) | ||
ghost = dcm.pixel_array[(ghost_row, ghost_col)] | ||
ghost = dcm.pixel_array[(ghost_col, ghost_row)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this an intentional change, swapping around ghost_col / ghost_row? Just checking in case you left if that way round during testing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Tom, thank you for all the comments. No, this one is not intentional.
img = dcm.pixel_array | ||
img = img.astype('float64') | ||
#print('this is img',img) | ||
img *= 255.0 / img.max() | ||
#img = hazenlib.rescale_to_byte(dcm.pixel_array) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The rescale_to_byte
function is also used in spatial_resolution.py
and possibly others. I'm a little concerned by changing it here and not doing wholesale across the entire repo. What do you think @laurencejackson ?
Lu – can you upload some images of img
generated using rescale_to_byte
and your new version, to help illustrate how the data differs? I just want to get a better sense of what's changed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Tom, the original rescale_to_byte function performs an histogram equalisation, which slightly manipulates the data. I think it is unnecessary to manipulate the data when we can just rescale in a more conventional way which does not manipulate the data. However, for this ghosting script, the rescale_to_byte is only used to plot, so actually I would leave the rescale_to_byte original function. I only changed this to check if it was making any difference while trying to fix the ghosting problem.
#figures = [] | ||
for dcm in data: | ||
try: | ||
key = f"{dcm.SeriesDescription.replace(' ', '_')}_{dcm.EchoTime}ms_NSA-{dcm.NumberOfAverages}" | ||
if report_path: | ||
report_path = key | ||
except AttributeError as e: | ||
print(e) | ||
key = f"{dcm.SeriesDescription}_{dcm.SeriesNumber}" | ||
try: | ||
fig, results[key] = get_ghosting(dcm, report_path) | ||
if report_path: | ||
fig.savefig(key + '.png') | ||
fig, results[key] = get_ghosting(dcm, report_path=True) | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't follow the changes here? Possibly linked to my previous comment about the image True.png
being output every time?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previously I could not get the script to output an image, so that's why I changed it, but now I need to fix this problem of always outputting an image that I created. I'll fix this.
tests/test_ghosting.py
Outdated
# assert -5.0 == hazen_ghosting.calculate_ghost_intensity(ghost=np.asarray([5]), | ||
# phantom=np.asarray([100]), | ||
# noise=np.asarray([10])) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can remove this – assume your abs() correction has made this redundant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes I think so too. Will remove then.
tests/test_ghosting.py
Outdated
with pytest.raises(Exception): | ||
hazen_ghosting.calculate_ghost_intensity(ghost=np.asarray([-10]), | ||
phantom=np.asarray([-100]), | ||
noise=np.asarray([-5])) | ||
phantom=np.asarray([-100]), | ||
noise=np.asarray([-5])) | ||
|
||
assert 5.0 == hazen_ghosting.calculate_ghost_intensity(ghost=np.asarray([10]), | ||
phantom=np.asarray([100]), | ||
noise=np.asarray([5])) | ||
phantom=np.asarray([100]), | ||
noise=np.asarray([5])) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some of the indentation in this section is awry – would you mind just tidying.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll tidy up the indentation.
tests/test_ghosting.py
Outdated
SIGNAL_BOUNDING_BOX = (164, 208, 166, 209) | ||
SIGNAL_CENTRE = [186, 187] | ||
BACKGROUND_ROIS = [(64, 187), (64, 140), (64, 93), (64, 46)] | ||
PADDING_FROM_BOX = 30 | ||
SLICE_RADIUS = 5 | ||
ELIGIBLE_GHOST_AREA = range(SIGNAL_BOUNDING_BOX[0], SIGNAL_BOUNDING_BOX[1]), range( | ||
SLICE_RADIUS, SIGNAL_BOUNDING_BOX[2] - PADDING_FROM_BOX) | ||
|
||
SIGNAL_SLICE = np.array(range(SIGNAL_CENTRE[0] - SLICE_RADIUS, SIGNAL_CENTRE[0] + SLICE_RADIUS), dtype=np.intp)[ | ||
:, | ||
np.newaxis], np.array(range(SIGNAL_CENTRE[1] - SLICE_RADIUS, SIGNAL_CENTRE[1] + SLICE_RADIUS), | ||
dtype=np.intp) | ||
GHOST_SLICE = np.array( | ||
range(min(ELIGIBLE_GHOST_AREA[1]), max(ELIGIBLE_GHOST_AREA[1])), dtype=np.intp)[:, np.newaxis], np.array( | ||
range(min(ELIGIBLE_GHOST_AREA[0]), max(ELIGIBLE_GHOST_AREA[0])) | ||
) | ||
PE = "COL" | ||
GHOSTING = (None, 0.015138960417776908) | ||
|
||
def setUp(self): | ||
self.file = str(TEST_DATA_DIR / 'ghosting' / 'PE_COL_PHANTOM_BOTTOM_RIGHT' / 'PE_COL_PHANTOM_BOTTOM_RIGHT.IMA') | ||
self.dcm = pydicom.read_file(self.file) | ||
def setUp(self): | ||
self.file = str( | ||
TEST_DATA_DIR / 'ghosting' / 'PE_COL_PHANTOM_BOTTOM_RIGHT' / 'PE_COL_PHANTOM_BOTTOM_RIGHT.IMA') | ||
self.dcm = pydicom.read_file(self.file) | ||
|
||
|
||
class TestAxialPhilipsBroomfields(TestGhosting): | ||
SIGNAL_BOUNDING_BOX = (217, 299, 11, 93) | ||
SIGNAL_CENTRE = [(SIGNAL_BOUNDING_BOX[0] + SIGNAL_BOUNDING_BOX[1]) // 2, | ||
(SIGNAL_BOUNDING_BOX[2] + SIGNAL_BOUNDING_BOX[3]) // 2] | ||
BACKGROUND_ROIS = [(258, 264), (194, 264), (130, 264), (66, 264)] | ||
PADDING_FROM_BOX = 30 | ||
SLICE_RADIUS = 5 | ||
ELIGIBLE_GHOST_AREA = range(SLICE_RADIUS, SIGNAL_BOUNDING_BOX[0] - PADDING_FROM_BOX), range( | ||
SIGNAL_BOUNDING_BOX[2], SIGNAL_BOUNDING_BOX[3]) | ||
SIGNAL_SLICE = np.array(range(SIGNAL_CENTRE[0] - SLICE_RADIUS, SIGNAL_CENTRE[0] + SLICE_RADIUS), dtype=np.intp)[:, | ||
np.newaxis], np.array(range(SIGNAL_CENTRE[1] - SLICE_RADIUS, SIGNAL_CENTRE[1] + SLICE_RADIUS), | ||
dtype=np.intp) | ||
GHOST_SLICE = np.array(range(11 - SLICE_RADIUS, 11 + SLICE_RADIUS), dtype=np.intp)[:, np.newaxis], np.array( | ||
range(16 - SLICE_RADIUS, 16 + SLICE_RADIUS)) | ||
SIGNAL_BOUNDING_BOX = (217, 299, 11, 93) | ||
SIGNAL_CENTRE = [(SIGNAL_BOUNDING_BOX[0] + SIGNAL_BOUNDING_BOX[1]) // 2, | ||
(SIGNAL_BOUNDING_BOX[2] + SIGNAL_BOUNDING_BOX[3]) // 2] | ||
BACKGROUND_ROIS = [(258, 264), (194, 264), (130, 264), (66, 264)] | ||
PADDING_FROM_BOX = 30 | ||
SLICE_RADIUS = 5 | ||
ELIGIBLE_GHOST_AREA = range(SLICE_RADIUS, SIGNAL_BOUNDING_BOX[0] - PADDING_FROM_BOX), range( | ||
SIGNAL_BOUNDING_BOX[2], SIGNAL_BOUNDING_BOX[3]) | ||
SIGNAL_SLICE = np.array(range(SIGNAL_CENTRE[0] - SLICE_RADIUS, SIGNAL_CENTRE[0] + SLICE_RADIUS), dtype=np.intp)[ | ||
:, | ||
np.newaxis], np.array(range(SIGNAL_CENTRE[1] - SLICE_RADIUS, SIGNAL_CENTRE[1] + SLICE_RADIUS), | ||
dtype=np.intp) | ||
GHOST_SLICE = np.array( | ||
range(min(ELIGIBLE_GHOST_AREA[1]), max(ELIGIBLE_GHOST_AREA[1])), dtype=np.intp)[:, np.newaxis], np.array( | ||
range(min(ELIGIBLE_GHOST_AREA[0]), max(ELIGIBLE_GHOST_AREA[0])) | ||
) | ||
|
||
GHOSTING = (None, 0.007246960909896829) | ||
|
||
def setUp(self): | ||
self.file = str(TEST_DATA_DIR / 'ghosting' / 'GHOSTING' / 'axial_philips_broomfields.dcm') | ||
self.dcm = pydicom.read_file(self.file) | ||
|
||
|
||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A lot of this indentation seems to be double-tabbed. Is that intentional? Github thinks the code has changed, but a lot of it looks the same from what I can tell.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll tidy up the indentation.
I fixed the problem where the script always outputted an image. Now it should only output if report_path == True
Hello @laurencejackson, @tomaroberts , I have fixed the ghosting script. The biggest problem was that the ghosting slice was always placed at random points in the image, not where it should have been, namely int the phase encoding direction. There is now a new function called get_ghosting_slice2 which selects the correct ghosting slice. I removed the old get_ghosting_slice function.
I have had to change the code slightly in the main function and in the report_path if statement because it didn't plot correctly.
Finally I changed the rescale_to_byte function because the original one changes the values and spreads them into a more widened histogram. I can put this back how it was but I think ideally that function will need to be changed at some point for all the hazenlib scripts.