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

Ellipsoid resulting model is bad #1082

Closed
akenmorris opened this issue Mar 3, 2021 · 32 comments
Closed

Ellipsoid resulting model is bad #1082

akenmorris opened this issue Mar 3, 2021 · 32 comments

Comments

@akenmorris
Copy link
Contributor

Results are terrible in 6.0-RC5. When did this happen?

6.0-RC5:

6.0Rc5-Ellipsoid.mp4

5.5:

5.5-Ellipsoid.mp4

Compare the variance chart:

6.0-RC5:

6 0rc5-ellipsoid

5.5:

5 5-ellipsoid

@akenmorris
Copy link
Contributor Author

It occurs to me that the ellipsoid dataset has changed. Can someone explain the difference between "v0" and "v1"? ShapeWorks 5.5 doesn't perform any better on the current ellipsoid dataset/parameters. @sheryjoe @jadie1 @iyerkrithika21

@iyerkrithika21
Copy link
Contributor

iyerkrithika21 commented Mar 3, 2021

@akenmorris "v1" is the un-groomed ellipsoid datasets generated using the ShapeCohortGenPackage. It includes all the following modes of variations and option for generations:

  1. random radii
  2. random orientation
  3. some segmentations which touch boundaries
  4. randomized centers
  5. randomize the size of the images to include more background

Whereas the old dataset had only one mode of variation, I suppose. @jadie1, correct me if I am wrong regarding v0.

@sheryjoe
Copy link
Contributor

sheryjoe commented Mar 3, 2021

@jadie1 We agreed before that we will provide two ellipsoid datasets, one aligned (similar to v0) and one not aligned (to demonstrate grooming). I think v1 is the latter.

@jadie1
Copy link
Contributor

jadie1 commented Mar 3, 2021

Yes ellipsoid-v1 is as @iyerkrithika21 described, so it should have more than one mode of variation. ellipsoid-v0 is the original data for which the ungroomed version is the same as the groomed.
We also added ellipsoid_aligned-v1 which was generated using the cohort generation code as ellipsoid-v1 was but it is already aligned (center, orientation, and image size are the same). The difference between ellipsoid-v0 and ellipsoid_aligned-v1 is ellipsoid-v0 only has one mode of variation whereas ellipsoid_aligned-v1 varies along all radii.

@sheryjoe
Copy link
Contributor

sheryjoe commented Mar 3, 2021

Yes ellipsoid-v1 is as @iyerkrithika21 described, so it should have more than one mode of variation. ellipsoid-v0 is the original data for which the ungroomed version is the same as the groomed.

Why should v1 have more modes? The use case for this data should factor out modes related to misalignment.

We also added ellipsoid_aligned-v1 which was generated using the cohort generation code as ellipsoid-v1 was but it is already aligned (center, orientation, and image size are the same). The difference between ellipsoid-v0 and ellipsoid_aligned-v1 is ellipsoid-v0 only has one mode of variation whereas ellipsoid_aligned-v1 varies along all radii.

This is becoming a bit confusing. Do we need to keep two aligned ellispoids data, one with one mode and the other with three modes? I think the aligned should only have one mode (pick one radius for variation) as this is supposed to be the simplest toy example for the optimization.

@akenmorris
Copy link
Contributor Author

Regardless of alignment or number of modes of variation, this set of parameters shows poor correspondence and sampling:

image

@jadie1
Copy link
Contributor

jadie1 commented Mar 3, 2021

Yes ellipsoid-v1 is as @iyerkrithika21 described, so it should have more than one mode of variation. ellipsoid-v0 is the original data for which the ungroomed version is the same as the groomed.

Why should v1 have more modes? The use case for this data should factor out modes related to misalignment.

Because the radius varies along all three axes there should be more modes of variation right?

We also added ellipsoid_aligned-v1 which was generated using the cohort generation code as ellipsoid-v1 was but it is already aligned (center, orientation, and image size are the same). The difference between ellipsoid-v0 and ellipsoid_aligned-v1 is ellipsoid-v0 only has one mode of variation whereas ellipsoid_aligned-v1 varies along all radii.

This is becoming a bit confusing. Do we need to keep two aligned ellispoids data, one with one mode and the other with three modes? I think the aligned should only have one mode (pick one radius for variation) as this is supposed to be the simplest toy example for the optimization.

I can remove ellipsoid_aligned-v1 or ellipsoid-v0. We don't have use cases that use both, I just left ellpsoid-v0 for users using older versions of ShapeWorks. This was the original request for creating ellpsoid_aligned-v1: "Can we regenerate v0 using the same code as v1 by disabling all transformations? This way we maintain consistency and have both segmentations and nice meshes to test both for fixed domains and cutting planes."

I am not sure why the model has poor correspondence. I guess the parameters need to be retuned? Should I try with new ellipsoids that only vary along one radius?

@akenmorris
Copy link
Contributor Author

akenmorris commented Mar 4, 2021

The grooming has problems as well:

image

Notice ellipsoid 9 is not even centered with the others.

Is this a problem in the ellipsoid example or in the Image library? @cchriste @archanasri @jadie1 @iyerkrithika21

@iyerkrithika21
Copy link
Contributor

This is how the groomed ellipsoids look for me in windows and Linux.
Ellipsoid 9 seems centered for me.

image

@jadie1
Copy link
Contributor

jadie1 commented Mar 4, 2021

Looks centered for me too - just ran it again on the release_v6.0 branch
image

@jadie1
Copy link
Contributor

jadie1 commented Mar 4, 2021

The correspondence points don't look great for me either though, maybe it needs more smoothing? To me it looks like there are four modes of variation - the first is size, and the next three are radii in the x, y, and z direction.

@akenmorris
Copy link
Contributor Author

@jadie1, @iyerkrithika21, I just tried on a different Mac, fresh install of shape works 6.0 RC5, new conda installs, everything. Look at the alignment for the samples (no need to even look at DTs):

ellipsoid.mp4

@akenmorris
Copy link
Contributor Author

akenmorris commented Mar 4, 2021

Oh, hah, @jadie1 , @iyerkrithika21 , you must turn off the center checkbox in Studio or it will auto center them for you for display purposes. Please inspect again with centering off.

@jadie1
Copy link
Contributor

jadie1 commented Mar 4, 2021

Oh yeah I see now. I just ran the getting-started-with-grooming-segmentations.ipynb which uses the same data and it seems to me it has the same problem but not as bad. See the last video here: http://sciinstitute.github.io/ShapeWorks/notebooks/getting-started-with-grooming-segmentations.html (this video matches what I just got running it on the release branch).

I guess we could go through the use case and notebook line by line and see where the grooming differs...

@jadie1
Copy link
Contributor

jadie1 commented Mar 5, 2021

Scratch that the notebook and use case distance transforms have the same center/alignment issue. The distance transforms from the notebook are just smoother.
@akenmorris do you think that the off centeredness may just be happening because there is a huge amount of variation in the dataset (size, orientation, radii, etc)?

@akenmorris
Copy link
Contributor Author

They are being aligned with ICP?

@jadie1
Copy link
Contributor

jadie1 commented Mar 5, 2021

Yes, the reference is ellipsoid_14

@akenmorris
Copy link
Contributor Author

akenmorris commented Mar 5, 2021

So do you think it ends up aligning ellipsoid 9 along only one side?

I'm not sure how we should address this. After ICP, should apply a center of mass operation?

@sheryjoe any thoughts? It seems wrong to capture this translation as part of the shape model.

@sheryjoe
Copy link
Contributor

sheryjoe commented Mar 5, 2021

@akenmorris which translation? I can't see this in the video. Did you QC the center of mass alignmenet step? Also, number of ICP iterations could matter in some cases.

@akenmorris
Copy link
Contributor Author

The ellipsoids are not centered, so necessarily the shape model captures the translation.

@sheryjoe
Copy link
Contributor

sheryjoe commented Mar 5, 2021

The ellipsoids are not centered, so necessarily the shape model captures the translation.

COM should center them.

@akenmorris
Copy link
Contributor Author

COM should center them.

I agree, but is ICP de-centering them to match one side?

@sheryjoe
Copy link
Contributor

sheryjoe commented Mar 5, 2021

COM should center them.

I agree, but is ICP de-centering them to match one side?

The impact of ICP should be minimal wrt translation after COM, unless the reference is not really a median shape.

@akenmorris
Copy link
Contributor Author

What's the status here? Is anyone working on this? I think there are two separate issues, grooming and then optimization parameters.

@sheryjoe
Copy link
Contributor

sheryjoe commented Mar 9, 2021

@jadie1 @iyerkrithika21 any update?

@iyerkrithika21
Copy link
Contributor

When I groomed the segmentations in Studio, the DTs looks this way:
image
So it looks like ICP could be causing some problems in the grooming.

@akenmorris
Copy link
Contributor Author

Correct. Ellipsoid 9 is centered before:

       """ Apply rigid alignment """
        ref = FindReferenceImage(comFiles)
        alignedFiles = applyRigidAlignment(groomDir + "aligned/segmentations", ref, comFiles)

And after, it is de-centered:

image

@iyerkrithika21
Copy link
Contributor

@akenmorris when I try to optimize the ellipsoids in Studio without ICP, I get this message.
image
I tried 3 different levels of padding - 10,30,50. But none of these helped.
And also in the python use case, I tried skipping the rigidAlignment step, and I get this bounding box error.

############## Cropping ##############
Traceback (most recent call last):
  File "RunUseCase.py", line 94, in <module>
    module.Run_Pipeline(args)
  File "/home/sci/iyerkrithika/ShapeWorks/Examples/Python/ellipsoid.py", line 96, in Run_Pipeline
    croppedFiles = applyCropping(groomDir + "cropped/segmentations", comFiles, comFiles)
  File "/home/sci/iyerkrithika/ShapeWorks/Examples/Python/GroomUtils.py", line 216, in applyCropping
    region = ImageUtils.boundingBox(bbDataList)
ValueError: Image sizes do not match (Output/ellipsoid/groomed/com_aligned/segmentations/ellipsoid_01.isores.center.com.nrrd)

This error doesn't make sense because the ImageUtils.boundingBox() should be able to give the largest bounding box.
@jadie1 do you think this could be related to the issue we saw in femur use case ?

@akenmorris
Copy link
Contributor Author

@iyerkrithika21 , can you tell me more about how you're running these in studio? If I had to guess based on the filenames in the corners, you perhaps imported the binary segmentations, skipped grooming and clicked run? They need to be distance transforms.

@iyerkrithika21
Copy link
Contributor

@iyerkrithika21 , can you tell me more about how you're running these in studio? If I had to guess based on the filenames in the corners, you perhaps imported the binary segmentations, skipped grooming and clicked run? They need to be distance transforms.

I imported the segmentations, ran groom in studio. and then clicked optimize.

@jadie1
Copy link
Contributor

jadie1 commented Mar 9, 2021

Will be fixed for release via issue #1133

@akenmorris
Copy link
Contributor Author

@iyerkrithika21 , I've added an issue for what you saw:

#1134

@sheryjoe sheryjoe mentioned this issue Mar 16, 2021
9 tasks
@jadie1 jadie1 closed this as completed Mar 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants