Contribute notebook on BSplineTransform

[zivy]

Original conversation on the SimpleITK repository, continued here.

[zivy]

The gist is nicer than what we currently have under BSpline in the transformations notebook.

Would be nice to combine the two, as the current BSpline illustration code is interactive. Possibly make the gist based code interactive too? Not sure if it should be added into the transformations noteook or replace the current BSpline example?

@fepegar, @jhlegarreta, what do you think?

[fepegar]

I agree that gist is asking to be interactive. I think it can be summarised enough to replace the current BSpline example.

[jhlegarreta]

Thanks both of you. The interactivity feature looks interesting. I will have a look into it as time permits and will try to summarize/simplify Fernando's gist piece of code and bring interactivity to it. I'll open a PR when I have a decent enough version, and will ping you both so that it can get comments and improvements.

[jhlegarreta]

Sorry for the delay @zivy. I had a look at the transformation notebook as suggested, got an idea of the current notebook's parts/flow, and was able to see its interactive feature.

Personally, I would not completely replace the current B-Spline transform part with the necessary changes to Fernando's code; I think the consistency and relative simplicity of the current tool is very valuable to see the BSpline transform compared to the rest of the tranforms.

Thus, I'd either add another section somewhere where Fernando's code fits with the necessary changes but without replacing what is in place. Maybe just before the Inverting bounded transforms section and naming the new part as e.g. Advanced insight into transforms and displacement fields or some other name that you might think of (since Fernando's example does not use the sitk::DisplacementFieldTransform class.).

Although I have not yet looked too much into it, I assume a custom display_displacement_scaling_effect method will be required to adapt to/host the new visualization, being the control points the interactive parameters as in the current examples.

If this makes sense, I will try to propose an addition along these lines and will ask for help in the corresponding PR if I get stuck at some point.

BTW related to the notebook: I'd say that the scale_factor_from_gui = statement in the second BSpline cell is missing its assignment value. If you let me know what a sensitive value is, I can push a patch set in a dedicated PR.

[zivy]

Hi @jhlegarreta ,

No worries. My conclusions and path forward:

1. Leave current code in place.
2. Add a section titled "Additional Insights into Bounded Transforms" under which you add the BSpline code (before the section on CompositeTransform and after the section on inverting bounded transforms). Once we have that I'll add something similar using DisplacementFieldTransform.
3. Implement a custom display_displacement_scaling_effect or if you really want to go nuts, display control points overlaid onto a 2D image, allow point dragging (see stackoverflow) and link the display update to that.

The scale_factor_from_gui = statement in the second BSpline cell is indeed missing its assignment value. This is intentional. Please take a second look at the comment above that statement. If it wasn't clear enough for you then the comment isn't sufficient. Let me know if you think the comment makes sense or if you have a clearer directive to the user. The intent should be clear to newcomers to SimpleITK (hard for both of us to do as we know what we're doing - at least we like to think so 😉 ).

Thanks again for all the help.

[jhlegarreta]

OK, thanks for the clarifications Ziv. I will try to give it a go in the coming days.

I'd leave adding point dragging capabilities for future PRs. I prefer to keep it simple for the first iteration.

As for the comment about the scale_factor_from_gui = now I realize the intent of the comment. Maybe something like

# Note that the scale value is left intentionally blank: set the scale value based on the slider value in the GUI above.
# You will get an error when executing the cell if a value is not provided.

would be clearer? Other comment statements do share the style of the current comment (e.g. # Concatenate all the information into a single list or # Set the interpolator, either sitkLinear which is default or nearest neighbor in the next cell), and the user is not expected to add code for those.

[zivy]

Updated the documentation in this PR.

Leaving the point dragging to future PRs makes sense. That is a bells and whistles addition. Always ask for the moon, you might just get it.