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

378 geom accuracy wrong slices #399

Merged
merged 10 commits into from
Jan 18, 2024
Merged

Conversation

mollybuckley
Copy link
Collaborator

@mollybuckley mollybuckley commented Jan 4, 2024

The ACR geometric accuracy test was measuring from slice 7 rather than slices 1 and 5 as per the ACR guidance. I have:

  • fixed this, so that the measurements were coming from the correct slices
  • combined the separate functions for ACR geometric accuracy slice 1 and 5 (the previous difference was that the slice 5 function was taking diagonal measurements too but the slice 1 function was not). The function now has slice index as an input and if the slice index is 4 then the diagonal measurements are made.

I then realised that the mask was being created incorrectly on slice 1 of the GE data (this was not previously an issue as the slice was being created from slice 7). This was because one of the pixel values outside the phantom was above the masking threshold value. I have changed the threshold value so that it is no longer picking up background noise pixels and including these in the mask. The images on the report look acceptable but some future work could be to test this with more GE data. I then changed three of the unit tests accordingly, as there were very minor variations in the results following this change.

The mask is now calculated from slices 1 and 5 (instead of 7) as per the ACR guidance.
The slice index (ie 0 for slice 1 etc) is now an input to the geometric accuracy function, rather than fixed at slice 7.

Also, get_geometric_accuracy_slice1 and get_geometric_accuracy_slice5 are combined into a generic get_geometric_accuracy function.
Changed syntax in unit tests to updated syntax in get_geometric_accuracy function
Corrections after some of the tests on Github failed (line too long, too many blank lines)
On slice 1 the mask was being generated incorrectly on the GE data (it was extending outside the phantom). Some pixels from outside the phantom had signal value above the masking threshold, so these were being picked up and incorporated in the mask. I increased the masking threshold from 0.05 to 0.07 and this meant that the background pixels were no longer being picked up.
changing the unit test values to the new correct values from slices 1 and 5
Following changes to the create mask function in ACR object, amendments to the acr ghosting and acr slice position unit tests were made to accommodate minor variation in results.
@mollybuckley mollybuckley marked this pull request as ready for review January 18, 2024 10:55
@sophie22
Copy link
Collaborator

resolves #378

hazenlib/ACRObject.py Show resolved Hide resolved
hazenlib/ACRObject.py Show resolved Hide resolved
hazenlib/ACRObject.py Show resolved Hide resolved
hazenlib/ACRObject.py Outdated Show resolved Hide resolved
hazenlib/tasks/acr_geometric_accuracy.py Show resolved Hide resolved
Slice 7 is needed for ghosting, uniformity and snr
Copy link

github-actions bot commented Jan 18, 2024

Coverage

Coverage Report
FileStmtsMissCoverMissing
hazenlib
   ACRObject.py1071190%92, 95–97, 102–105, 143, 197–200
   HazenTask.py29390%43–47
   __init__.py571574%173, 205–214, 216–225, 227–229, 246–250, 254
   exceptions.py21576%19–23, 42
   utils.py1894377%72, 76, 86, 91, 130, 137–150, 161, 168–175, 195–197, 215–219, 238–242, 251, 256, 267, 319, 322, 330–335, 338, 385, 394, 409
hazenlib/tasks
   acr_geometric_accuracy.py1125848%57–97, 119–180
   acr_ghosting.py1074261%46–62, 105–107, 159–161, 211–293
   acr_slice_position.py1364865%59–83, 286–353
   acr_slice_thickness.py1366056%48–67, 238–322
   acr_snr.py1345857%47, 62–113, 132, 227–242, 283–301, 348–373
   acr_spatial_resolution.py2076867%69–100, 188, 286, 303–314, 460–539
   acr_uniformity.py813260%47–64, 148–200
   ghosting.py1495166%28–47, 67, 171–172, 179, 196–197, 252–256, 271–275, 346–387
   relaxometry.py2918969%210–211, 213, 226–231, 238–246, 277–326, 375, 409–431, 609, 655–659, 726, 811–833, 851–866
   slice_position.py1244068%30, 43–71, 129–130, 157, 273, 283–306
   slice_width.py3525285%44–48, 52, 123, 188–213, 555, 560–561, 567, 572, 648–649, 1021–1085
   snr.py1736960%45–48, 87, 103–113, 206–225, 237–247, 287–302, 330–340, 345–361, 399–415, 428–434, 477–495
   snr_map.py108199%157
   spatial_resolution.py2464482%50–54, 58, 90, 213, 294, 460–503
   uniformity.py801976%59–63, 67, 118–119, 126, 175–205
TOTAL286480872% 

Tests Skipped Failures Errors Time
198 0 💤 0 ❌ 0 🔥 2m 20s ⏱️

simplify variables
@sophie22 sophie22 self-requested a review January 18, 2024 15:05
Copy link
Collaborator

@sophie22 sophie22 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! All comments have been satisfactorily addressed.
Happy to merge :)

@sophie22 sophie22 merged commit 8cb6f66 into main Jan 18, 2024
4 checks passed
@sophie22 sophie22 deleted the 378-geom-accuracy-wrong-slices branch January 18, 2024 15:17
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.

2 participants