-
Notifications
You must be signed in to change notification settings - Fork 14
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
ENH: added Elastix registration (issue #334) #341
base: master
Are you sure you want to change the base?
Conversation
* SliceTrackerRegistrationLogic expects registration algorithm as a parameter in order to be interchangeable * IRegistrationAlgorithm is the interface that needs to be implemented in order to add new registration algorithm * new configuration entries * adapted SliceTrackerRegistration widget and cli * gentle handling if preferred algorithm doesn't exist with giving notification and using fallback algorithm (default: BRAINS) TODO: use own parameter files needs to possible
I would change |
Should there be an algorithm-specific implementation of the resampling method? |
I think it makes sense for the abstract class to have functions that define the output required from the algorithm. As is, it is unclear what are the expectations from |
Yeah I can do that
I don't know which resampling method you mean
Technically you can always expect an output volume and an output transform. Both are stored in the RegistrationResult instance. Maybe I should decouple it from RegistrationResult? |
Do you mean the part, where I am applying the initial rigid registration transform to the moving label/volume? |
I thought that elastix does not produce a Slicer node for the deformable transform, and we need to use transformix to apply it to the dataset, that's what I meant. |
In SlicerElastix you can select an output transform which is an vtkMRMLTransformNode. It works in SliceTracker this way, but there are some issues with elastix registration so we need to look into the parameter files. |
@fedorov That's probably what you were looking for: https://github.com/lassoan/SlicerElastix/blob/master/Elastix/Elastix.py#L577-L586 |
…state#334) * only using Parameters_BSpline.txt for now
@fedorov, @Kmacneil0102 feel free to play with the parameter files. For now I only activated the BSpline_Parameters.txt to be used, but also tried a combination of rigid, affine and bSpline. All three parameter files were added with commit 768f6dd CLI execution
The combination though gives me errors:
Maybe helpful resources: |
@che85, as we discussed and tested at the project week, elastix initializer is not good enough to deal with this kind of data. You should use BRAINS CenterOfROI initializer, and then pass the resulting transform to elastix. |
@fedorov That's what I did, when you look at [1].
Am I missing something or doing anything wrong? If so, I will look into that next week when I get back. [1] https://github.com/che85/SliceTracker/blob/integrate_Elastix/SliceTracker/SliceTrackerUtils/algorithms/registration.py#L236-L247 |
I didn't try. Is it the same dataset we used at the PW? |
In your code, you run rigid registration after the ROI initialization. At the PW we just ran the initializer, and passed the result to elastix. |
You are right. I somehow memorized that I clicked the rigid checkbox in the BRAINS registration. Will correct that now. |
Some more information regarding how we addressed the integration of Elastix: Elastix doesn’t support using the center of ROI( in Elastix it’s called mask) align. That’s why we choose the following approach to be used. PipelineBRAINSFit ROI initialization → transform moving label and volume → using generic (all) preset that includes rigid and BSpline parameter files → running registration In the first place, rigid registration failed sometimes, so we decided to change the parameter file for the rigid registration. Parameters_Rigid.txt
After changing the parameters, I was not able to reproduce the errors, that we had earlier. |
Didn't realize this - but this PR being open means Elastix has not been integrated, right? |
@fedorov It would be interesting which version we are using right now in production. This PR never got merged |
Indeed, too bad it didn't get merged while you were still here. Now I don't want to do it not to mess up something! I should check which version we are using. |
fixes #334
parameter in order to be interchangeable
in order to add new registration algorithm
notification and using fallback algorithm (default: BRAINS)
TODO: use own parameter files needs to possible
Class diagram