Skip to content
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

Optimise DicomImage #547

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

alanphys
Copy link
Contributor

Hi James

I was looking through DicomImage in image.py and I saw that DicomImage.__init__ had

        self.metadata = retrieve_dicom_file(path)
        self._original_dtype = self.metadata.pixel_array.dtype
        self._raw_pixels = raw_pixels
        # read a second time to get pixel data
        try:
            path.seek(0)
        except AttributeError:
            pass
        ds = retrieve_dicom_file(path)
        if dtype is not None:
            self.array = ds.pixel_array.astype(dtype)
        else:
            self.array = ds.pixel_array.copy()
        # convert values to HU or CU
        self.array = _rescale_dicom_values(self.array, ds, raw_pixels=raw_pixels)

This seems a bit redundant to me. The old adage "If it aint broke doan fix it" probably applies and I'm sure there are good reasons why it is this way, but if I remove the second file load giving:

        self.metadata = retrieve_dicom_file(path)
        self._original_dtype = self.metadata.pixel_array.dtype
        self._raw_pixels = raw_pixels
        if dtype is not None:
            self.array = self.metadata.pixel_array.astype(dtype)
        else:
            self.array = self.metadata.pixel_array.copy()
        # convert values to HU or CU
        self.array = _rescale_dicom_values(self.array, self.metadata, raw_pixels=raw_pixels)

I get quite a significant performance increase. I tested using:

from pylinac import CatPhan604
import time

for I in range(5):
    start = time.time()
    ct = CatPhan604('../CatPhan604/')
    end = time.time()
    print(end - start)
    del ct

I get for the original function
0.37560391426086426
0.3352365493774414
0.33795595169067383
0.3667294979095459
0.33728504180908203

and for the modified function
0.30944371223449707
0.26961445808410645
0.2696726322174072
0.29987263679504395
0.2696547508239746

for an 88 slice CBCT.

Unfortunately I was not able to run the unit tests due to issue #543 to double check this does not affect anything down stream, but my test sets loaded and analyzed correctly. I realise this is quite a fundamental change but I thought I would put it out there.

Regards
Alan

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

Successfully merging this pull request may close these issues.

1 participant