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

Transformation on Subvolume for mri3d_subvolume_segmentation_dataset #1169

Merged
merged 6 commits into from
Jul 3, 2022

Conversation

cakester
Copy link
Contributor

@cakester cakester commented Jun 16, 2022

Checklist

GitHub

  • I've given this PR a concise, self-descriptive, and meaningful title
  • I've linked relevant issues in the PR body
  • I've applied the relevant labels to this PR
  • I've assigned a reviewer

PR contents

Description

transformation on subvolume for mri3d_subvolume_segmentation_dataset instead of full volume

Linked issues

resolves #993

@cakester cakester requested a review from dyt811 June 16, 2022 16:22
@cakester
Copy link
Contributor Author

@coveralls
Copy link

coveralls commented Jun 16, 2022

Pull Request Test Coverage Report for Build 2605683149

  • 39 of 44 (88.64%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.08%) to 74.153%

Changes Missing Coverage Covered Lines Changed/Added Lines %
ivadomed/loader/mri3d_subvolume_segmentation_dataset.py 26 31 83.87%
Totals Coverage Status
Change from base Build 2578332769: 0.08%
Covered Lines: 4530
Relevant Lines: 6109

💛 - Coveralls

Copy link
Member

@lifetheater57 lifetheater57 left a comment

Choose a reason for hiding this comment

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

If I understand correctly, the current code changes don't change what is transformed are still the full size pairs (Related to @dyt811 comment.).

Also, if I understood correctly, previously we would have:

1. Take full volume -> 2. Apply transformation -> 3. Take subvolume -> 4. Do something with subvolume

Is the intent of this PR to get the same transformed subvolume to do 4?

If we do:

1. Take subvolume -> 2. Apply transformation -> 3. Do something with subvolume

Depending on the transformation applied, we may get a different result because some transformations are sensitive to the change in coordinates.

To, keep the same transformation given the change in coordinates, we might want to do something like:

1. Take subvolume -> 2. Transform the transformation -> 3. Apply transformed transformation -> 4. Do something with subvolume

ivadomed/loader/mri3d_subvolume_segmentation_dataset.py Outdated Show resolved Hide resolved
@dyt811
Copy link
Member

dyt811 commented Jun 27, 2022

Lucy's feedback a few days ago posted on Slack:

The order of image processing pipeline of doing subvolume transformation instead of full volume transformation do affect the transformation result. For spacial transfer such as flip, rotate, etc, the result wouldn’t change. But for image smoothing/blur, elastic-transfromation, etc, because a convolution kernel is used, the transformation result is different. For ML training propers, I don’t think this difference matters. However, if the transformation is k-space transform, you definitely CANNOT apply k-space transformation on subvolume.

So overall, I agree that we may have to make this path mandatory and full volume augmentation optional?

@dyt811 dyt811 self-assigned this Jul 1, 2022
Copy link
Member

@lifetheater57 lifetheater57 left a comment

Choose a reason for hiding this comment

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

I approve this PR since there is no more functional work to do given my understanding. However, I suggest to do the changes related to the keywords usage for maintainability reasons and the changes related to the ranges to ease the code analysis.

@dyt811 dyt811 merged commit a588984 into master Jul 3, 2022
@dyt811 dyt811 deleted the hm/subvolume_3d branch July 3, 2022 16:52
@mariehbourget mariehbourget added the enhancement category: improves performance/results of an existing feature label Oct 31, 2022
@mariehbourget mariehbourget added this to the new release milestone Oct 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement category: improves performance/results of an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Transforms in 3D are applied on full volumes instead of sub-volumes
5 participants