voxelmorph / voxelmorph

Unsupervised Learning for Image Registration
Apache License 2.0
2.25k stars 576 forks source link

PyTorch Redesign #170

Closed kleingeo closed 4 years ago

kleingeo commented 4 years ago

I know that at the moment the Voxelmorph project is being updated for PyTorch, and looking at the Redesign branch it looks like there is significant progress. I am currently trying to reimplement the Miccai2018 model in PyTorch, but noticed that the model for it is not fully scripted. I have no problem designing the model myself based on the older Keras/TF model, but I was wondering if the VecInt and SpatialTransformer layers are fully functional.

adalca commented 4 years ago

@kleingeo sorry for missing this earlier. Did you end up making progress on this? I don't believe we had the VecInt layers, although it was probably easy to creat?

kleingeo commented 4 years ago

No, I did not make any progress with it. But I have been testing the PyTorch code with some data that did work (well, train to some degree) with the Keras/Tensorflow model. However, it did not train at all with the PyTorch version (the loss function did not decrease at all). I thought there was a VecInt layer in the PyTorch code, but looking at it more, it seems to just call the SpatialTransformer function.

Is there another reason why the PyTorch code is not working properly? I have not delved too deep into the specifics (go through both the Keras/TF and compare with the PyTorch line-by-line to look for differences) but that may be the only way for me to determine the potential issue. It does seem to be "training" in the sense that the loss function is not constant, but it is only changing a tiny few decimal places. By that, I mean the model is at least set to model.training = True and not model.eval(). However, it is not performing as it should be when compared to the Keras/TF version.

adalca commented 4 years ago

We can take a look into this, which pytorch version are you using, the master or redesign branch?

@balakg can probably comment more on the pytroch implementation in the master branch

kleingeo commented 4 years ago

I am using the redesign since I want to take advantage of the bidir.

adalca commented 4 years ago

okay I see. I can try to look at this but can't super soon. @ahoopes and/or @balakg might you be able to take a quick look? I'm suspecting a simple bug in the translation of master to redesign. The master gave good results on our data.

@kleingeo if you happen to track down the bug help would be appreciated

ahoopes commented 4 years ago

I can take a look at this later today

ahoopes commented 4 years ago

Closing as this should be accounted for in the current master branch. The VecInt layers in voxelmorph/torch/layers.py are fully functional, and training with diffeomorphism has been tested successfully in our pytorch implementation. Please feel free to reopen this if you are still having troubles!