Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

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

Open
mitch-1211 opened this issue Sep 12, 2024 · 4 comments

Comments

@mitch-1211
Copy link

mitch-1211 commented Sep 12, 2024

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
Copy link
Author

mitch-1211 commented Sep 12, 2024

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
Copy link
Author

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
Copy link
Owner

jrkerns commented Sep 12, 2024

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
Copy link
Author

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

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants