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

New script to compute similarity between mask and atlas (nii) #994

Merged
merged 9 commits into from
Aug 9, 2024

Conversation

frheault
Copy link
Member

@frheault frheault commented May 22, 2024

Quick description

This is similar to scil_bundle_pairwise_comparison.py, but for mask and atlas (as nifiti). This is an easy way to quantify the similarity between wmparc of bundle label maps or BET mask in scan-rescan or across subjects for example.

This is generating a JSON file that contains four measures: dice, adjacency_voxels, overlap and overreach.

Everything has to be in uint8 or uint16, coregistered nifti.

...

Type of change

Check the relevant options.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Provide data, screenshots, command line to test (if relevant)

...

Checklist

  • My code follows the style guidelines of this project (run autopep8)
  • I added relevant citations to scripts, modules and functions docstrings and descriptions
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I moved all functions from the script file (except the argparser and main) to scilpy modules
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Copy link

codecov bot commented May 22, 2024

Codecov Report

Attention: Patch coverage is 27.51678% with 108 lines in your changes missing coverage. Please review.

Project coverage is 68.31%. Comparing base (175eb67) to head (cd5e6c4).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #994      +/-   ##
==========================================
- Coverage   68.61%   68.31%   -0.30%     
==========================================
  Files         426      427       +1     
  Lines       21834    21940     +106     
  Branches     3267     3286      +19     
==========================================
+ Hits        14981    14989       +8     
- Misses       5578     5676      +98     
  Partials     1275     1275              
Components Coverage Δ
Scripts 69.47% <7.31%> (-0.46%) ⬇️
Library 66.64% <52.23%> (-0.06%) ⬇️

Copy link
Contributor

@AntoineTheb AntoineTheb left a comment

Choose a reason for hiding this comment

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

First pass, broad comments

img_1, dtype_1 = load_img(filename_1)

if np.issubdtype(dtype_1, np.floating):
raise ValueError('Input file {} is of type float.'.format(filename_1))
Copy link
Contributor

Choose a reason for hiding this comment

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

Why only float ? The comments in the header of the script mention that the inputs must be uint8 or uint16, but here int32 would pass the test.

Copy link
Member Author

Choose a reason for hiding this comment

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

This was simply to make the error message a bit clearer in the context of this script (if used from Freesurfer atlas for example)

The function get_data_as_labels() will convert int32 to uint16 automatically

binary_2[data_2 == val] = 1

# These measures are in mm^3
volume_overlap = np.count_nonzero(binary_1 * binary_2)
Copy link
Contributor

Choose a reason for hiding this comment

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

See if you could use:
scilpy.tractanalysis.scoring.compute_f1_overlap_overreach, which can be tested with unit tests.

Copy link
Member Author

Choose a reason for hiding this comment

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

Since this does not always use , it is not always normalized between 0 - 1 (this script is not for GT). So I will leave it like that.

@pep8speaks
Copy link

pep8speaks commented Jul 25, 2024

Hello @frheault, Thank you for updating !

Line 224:80: E501 line too long (80 > 79 characters)

Line 50:80: E501 line too long (80 > 79 characters)

Comment last updated at 2024-08-05 18:09:41 UTC

@frheault
Copy link
Member Author

@ThoumyreStanislas Could you try this branch before merging to compare atlas (labels of bundle in scan-rescan, I think you have penthera) and maybe lesions masks?

That would be helpful to improve the docstring

Copy link
Contributor

@ThoumyreStanislas ThoumyreStanislas left a comment

Choose a reason for hiding this comment

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

script works just as well for masks as for bundle labels, the only thing is that json is not the easiest to understand. In particular, when comparing binary masks with the “1” section in the json and the order of inputs in each section, even if we can guess it, it might be good to explain exactly how the classification works in the json.

Copy link
Contributor

@arnaudbore arnaudbore left a comment

Choose a reason for hiding this comment

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

A couple of things to fix, but we are getting closer !

@frheault frheault merged commit 9ab3ce4 into scilus:master Aug 9, 2024
2 checks passed
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.

6 participants