Skip to content

Overall atlas segmentation pipeline in Python#69

Merged
dzenanz merged 7 commits intoKitwareMedical:masterfrom
dzenanz:master
Feb 27, 2022
Merged

Overall atlas segmentation pipeline in Python#69
dzenanz merged 7 commits intoKitwareMedical:masterfrom
dzenanz:master

Conversation

@dzenanz
Copy link
Copy Markdown
Member

@dzenanz dzenanz commented Feb 25, 2022

No description provided.

As ApplyToImageMetadata is not available in Python and might never be,
remove the commented-out code which mentions it.
See this commit for an attempt to make it available in Python:
dzenanz/ITK@db8157e
@dzenanz dzenanz requested review from tbirdso and thewtex February 25, 2022 16:57
@review-notebook-app
Copy link
Copy Markdown

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

Copy link
Copy Markdown
Contributor

@tbirdso tbirdso left a comment

Choose a reason for hiding this comment

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

Code is looking good overall. A few questions in regards to the vision for this code and how/whether it will need to be generalized for arbitrary sample sets.

Comment on lines +121 to +122
# we don't need to change laterality of atlas landmarks
# as they all lie in a plane with K coordinate of zero
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not sure I fully understand the geometry here. Will this generalize to other sample sets?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It won't. But I guess I could (and should) apply some permutation matrix transform on the atlases (and some landmarks files) so the mirroring here is along LR axis instead of SI. Thanks for reminding me of this.

# we don't need to change laterality of atlas landmarks
# as they all lie in a plane with K coordinate of zero

atlas_bone_label_filename = root_dir + bone + '/' + atlas + '-AA-' + bone + '-label.nrrd'
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this filename convention documented and enforced elsewhere in the project as well?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It is probably not documented yet. This is just where I have put files on my computer. I guess we should document it in the future, or make it configurable.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Overall, we should move away from file IO, file name conventions, to in-memory data structure input output, dagster.

Comment on lines +211 to +223
label_names = {
1: 'Diaphysis',
2: 'Metaphysis',
3: 'Growth Plate',
4: 'Epiphysis',
}
# TODO: add proper reading of labels from .seg.nrrd files so this is not hardcoded
# label_names = {
# 1: 'Cortical',
# 2: 'Trabecular VOI',
# 3: 'New Trabecular VOI',
# 4: 'New Cortical VOI',
# }
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is there a strategy for revisiting this later? Possibly accepting an arbitrary list of label names?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I was hoping to be able to read label names from .seg.nrrd files, which would make this hard-coded list unnecessary. That would also involve less manual steps, and therefore be less error-prone. I was planning to do this for the full set of atlases (6 per bone).


if __name__ == '__main__':
if len(sys.argv) == 1: # direct invocation
atlas_list = ['901-L', '901-R', '907-L', '907-R', '917-L', '917-R', 'F9-3wk-02-L', 'F9-3wk-02-R']
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Perhaps not necessary to change for this PR, but would be good to clarify what steps will be needed to generalize pipeline for arbitrary samples. We probably would not want a developer to have to import these lines even if they never run __main__ directly.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Currently, this is the entry-point for the analysis. We plan to use https://github.com/dagster-io/dagster later, so this will have to change too.

Copy link
Copy Markdown
Contributor

@thewtex thewtex left a comment

Choose a reason for hiding this comment

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

Good progress, more todo to make it more general and move away from file IO.

Before merging, please rename the file from segmentation_pipeline.py to something more specific, e.g. mouse_femur_ct_morphometry.py

Handles laterality:
Atlas and case under consideration can have different laterality
(e.g. atlas is from right leg, case under consideration from left leg),
so we appropriately mirror the atlas.

Crop atlas to the bounding box of the labeled region (bone in question)
Crop case to the bounding box of the bone in question
Without this, elastix crashes on the second
iteration of the loop `for case in data_list`.
Compress bone atlas
As atlases are already axis-aligned, we don't need its landmarks
@dzenanz dzenanz merged commit 0f50ab5 into KitwareMedical:master Feb 27, 2022
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.

3 participants