jrkerns / pylinac

An image analysis library for medical physics
https://pylinac.readthedocs.io/en/latest/
MIT License
155 stars 98 forks source link

computed image metrics returning last metric calculated rather than entire dictionary #517

Open mitch-1211 opened 1 month ago

mitch-1211 commented 1 month ago

Describe the bug using image metrics with SizedDiskLocator to find BBs in an image. Only the last metric to be evaluated is returned in the dictionary. The dictionary should contain the results of all metrics evaluated.

To Reproduce Steps to reproduce the behavior: Run the following code:

from pylinac.metrics.image import SizedDiskLocator
from pylinac.core.image import DicomImage, Point

path = "path_to_dicom_image"

img = DicomImage(path)

p1 = Point(0,0)
p2 = Point(0,-57)

bb_locations = img.compute(
        metrics=[
            SizedDiskLocator.from_center_physical(
                expected_position_mm=p1,
                search_window_mm=(7,7),
                radius_mm=3,
                radius_tolerance_mm=2
            ),
            SizedDiskLocator.from_center_physical(
                expected_position_mm=p2,
                search_window_mm=(7,7),
                radius_mm=3,
                radius_tolerance_mm=2
            )
        ]
    )

print(bb_locations)

bb_locations is a dictionary containing a single entry

Expected behavior bb_locations is a dictionary containing the points of BBs that were close to p1 and p2

mitch-1211 commented 1 month ago

So under metrics/image.py we see that in the definition of the SizedDiskRegion class, name is set to a constant string value: name: str = "Disk Region"

Under core/image.py in the compute function, we see how the dictionary is populated:

for metric in metrics:
            metric.inject_image(self)
            value = metric.context_calculate()
            self.metrics.append(metric)
            metric_data[metric.name] = value

So as metric.name is not a unique key (name: str = "Disk Region"), we see the value get overwritten as each metric of the same type is calculated, resulting in a single value returned in the dictionary that corresponds to the last metric evaluated.

This would not be an issue if different types of metrics were calculated, e.g. combining a SizedDiskLocator with a GlobalDiskLocator

mitch-1211 commented 1 month ago

There is a workaround, but it just requires more lines of code:

def get_bb_location(estimate_point):
    bb_location = img.compute(
        metrics=[
            SizedDiskLocator.from_center_physical(
                expected_position_mm=estimate_point,
                search_window_mm=(7,7),
                radius_mm=3,
                radius_tolerance_mm=2
            )
        ]
    )
    return bb_location

# +ve Y = lower in image
# +ve X = right side of image

bb_estimates = [
    Point(0,0),     # centre [0]
    Point(0,57.5),  # bottom inner [1]
    Point(0,-57.5), # top inner [2]
    Point(57.5,0),  # right inner [3]
    Point(-57.5,0), # left inner [4]
    Point(0,107.5), # bottom outer [5]
    Point(0,-107.5),# top outer [6]    
    Point(107.5,0), # right outer [7]
    Point(-107.5,0) # left outer [8]
]

bb_locations = []

for estimate in bb_estimates:
    bb_locations.append(get_bb_location(estimate)[0])
jrkerns commented 1 month ago

Yup, known issue that's still outstanding. https://forum.pylinac.com/t/retrieving-multiple-metrics-information/419. I will work on it early next week. Thanks!

mitch-1211 commented 1 month ago

Thanks @jrkerns didn't realise you could override the metric name, that will get me out of trouble for now