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

Remove redundant PyTorch functionality #15

Open
jacobhinkle opened this issue Dec 11, 2018 · 0 comments
Open

Remove redundant PyTorch functionality #15

jacobhinkle opened this issue Dec 11, 2018 · 0 comments
Labels
enhancement New feature or request

Comments

@jacobhinkle
Copy link
Owner

Is your feature request related to a problem? Please describe.
Basically, some of the main work in lagomorph was already implemented by the pytorch team. I was unaware of some of it when I started. In particular, regridding, affine grids, and interp are all provided in the torch.nn.functional library. Those are probably better engineered and we should use them if so.

Describe the solution you'd like
Once #8 is addressed and we have a benchmarking suite, we should compare against built-in pytorch methods where they are available.

Describe alternatives you've considered
Reducing clutter in lagomorph would be a good thing, but there may be reasons to keep some of the redundant functionality. For example, in order to use affine transforms in pure pytorch, you have to create some intermediate grids which really are unnecessary, leading to lots of wasted memory. However, in cases where my approach does not have an advantage, I am all for getting rid of my version.

Additional context
There is still functionality lagomorph provides for computational anatomy beyond what is in pure pytorch.

In particular, it is impractical at the moment to implement a fluid kernel in pytorch as far as I know, but contemplating that might be a useful exercise. For example, can the cholesky factor be saved as a 6-channel image and applied efficiently with slicing operations? Again this will need some benchmarking.

Another similar case is jacobian times vectorfield. We could use slicing and roll(), but that's probably going to be slower than my current approach, which is admittedly pretty complicated due to needing the adjoint transform.

@jacobhinkle jacobhinkle added the enhancement New feature or request label Dec 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant