imglib / imglib2-realtransform

Spatial transformations for ImgLib2
Other
7 stars 7 forks source link

ThinPlateSplineTransform : make the kernel accessible from the transform ? #30

Closed NicoKiaru closed 2 years ago

NicoKiaru commented 3 years ago

Hi everybody,

Do you think it could be possible to add a method which would return the landmarks (moving and fixed points) from the ThinPlateSplineTransform class, possibly via the inner kernel ?

Maybe there's a good reason not to make them accessible ? Through reflexion it's possible to access the fixed points via the ThinPlateR2LogRSplineKernelTransform kernel, but the moving points are too deeply buried into the kernel.

This would help with serialization.

bogovicj commented 3 years ago

I'm happy to make more internals accessible, but one issue I see is that the implementation does not store the fixed points because they're not needed to apply the transformation. E.g. this constructor takes the moving points and parameters that are derived from fixed points.

If one needs fixed points, one has to store them at creation of the ThinPlateSpline if they're needed for some other purpose. Would a getter for the wrapped ThinPlateR2LogRSplineKernelTransform be helpful?

NicoKiaru commented 3 years ago

Thanks for the quick feedback! I need to digest this information, rethink a bit then I'll let you know ;-).

NicoKiaru commented 3 years ago

If one needs fixed points, one has to store them at creation of the ThinPlateSpline if they're needed for some other purpose.

The issue is that I do not necessarily have the control when the ThinPlateSplineTransform is created... So I cannot catch easily the landmarks on creation.

I can describe my problem in more details, if you have an idea. It consists of two points:

  1. I want to be able to serialize and deserialize a ThinPlateSplineTransform object.
  2. I want to be able to edit the ThinPlateSplineTransform object after deserialization ( not necessarily mutating it ), by launching BigWarp with the retrieved landmark (is there any other way ?)

For serialization, I can get the kernel through reflection (a getter would be helpful indeed). And the kernel can be serialized directly.

For deserialization, I can instantiate a ThinPlateSplineTransform using the constructor that takes the deserialized kernel.

Overall this solves the point 1.

Now I do not have a way to solve the second point using this method, because I cannot retrieve landmarks with this way serializing a ThinPlateSplineTransform. Ideally if there was a getter method for fixed and moving points within the ThinPlateSplineTransform that would be easier. But they do not necessarily exist ? ...

If you don't see an easy way of solving the point 2, then I can try to catch the landmarks when they are created and see how far I can go.

bogovicj commented 3 years ago

For serialization, I can get the kernel through reflection (a getter would be helpful indeed). And the kernel can be serialized directly.

Agree, we should add a getter.

Here's my recommendation for point (2) for now; given that you can:

I would recommend getting the target points by applying the transforms to all of the source points in turn. There will be errors, but they will be tiny (near machine precision, hopefully) for "reasonable" transformations.

I'll do some thinking about what it would take to make this easier.

NicoKiaru commented 3 years ago

Ahhh, because it's invertible, I can retrieve the fixed points! I didn't think about it, thanks a lot!!

NicoKiaru commented 3 years ago

@bogovicj Ok I didn't realized it was so simple : just directly applying the landmarks to the kernel, no inverse required at all.

Ok, so my issue is solved. I renamed the issue : only the kernel is required.

Should I close the issue ? Or let it open to keep the discussion about the kernel getter ?

bogovicj commented 3 years ago

just directly applying the landmarks to the kernel, no inverse required at all. Ok, so my issue is solved.

Fantastic! That's right.

The only case I can think of where there would be issues are silly, ridiculous corner cases (e.g. the same moving point to multiple, different target locations), but who would do such a thing?!

Or let it open to keep the discussion about the kernel getter ?

Let's leave it open for this reason, I think having the getter is a good idea.