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

Rearchitect F3D to reinstate GPU #92

Open
onurulgen opened this issue Nov 11, 2022 · 10 comments · May be fixed by #114
Open

Rearchitect F3D to reinstate GPU #92

onurulgen opened this issue Nov 11, 2022 · 10 comments · May be fixed by #114
Assignees

Comments

@onurulgen
Copy link
Member

No description provided.

@onurulgen onurulgen self-assigned this Nov 11, 2022
@onurulgen onurulgen moved this to In Progress in NiftyReg Revival Nov 11, 2022
@onurulgen onurulgen changed the title Refactoring GPU for F3D Rearchitect F3D to reinstate GPU Nov 11, 2022
@mmodat
Copy link
Contributor

mmodat commented Jun 21, 2023

@onurulgen As mentioned earlier - I am finally able to allocate some time to NiftyReg.

Looking at differences between this branch and master, it appears some discrepancies happen when using lncc as a measure of similarity between the two branches. I will add relevant tests to pinpoint the potential error(s).

mmodat added a commit that referenced this issue Jun 23, 2023
mmodat added a commit that referenced this issue Jul 7, 2023
@mmodat
Copy link
Contributor

mmodat commented Jul 12, 2023

@onurulgen When you have a chance, could you go through the list of what still needs to be done before we finalise this branch and merge it back?
Please add it as a list of tasks and we can get cracking.

@onurulgen
Copy link
Member Author

@onurulgen As mentioned earlier - I am finally able to allocate some time to NiftyReg.

Looking at differences between this branch and master, it appears some discrepancies happen when using lncc as a measure of similarity between the two branches. I will add relevant tests to pinpoint the potential error(s).

Fixed in e6855af

@onurulgen
Copy link
Member Author

onurulgen commented Jul 13, 2023

The symmetric scheme isn't supported for these classes:

  • reg_nmi_gpu
  • reg_ssd_gpu
  • reg_optimiser_gpu
  • reg_conjugateGradient_gpu

CUDA implementations needed:

  • CudaCompute::GetDefFieldFromVelocityGrid()
  • CudaCompute::ApproxLinearEnergy()
  • CudaCompute::ApproxLinearEnergyGradient()
  • CudaCompute::SmoothGradient()
  • CudaCompute::ConvolveVoxelBasedMeasureGradient()
  • CudaCompute::ExponentiateGradient()
  • CudaCompute::SymmetriseVelocityFields()
  • CudaCompute::UpdateVelocityField()
  • reg_lncc_gpu
  • reg_optimiser_gpu::Perturbation()
  • CudaCompute::GetLandmarkDistance()
  • CudaCompute::LandmarkDistanceGradient()
  • CudaCompute::GetApproximatedGradient()
  • CudaCompute::BchUpdate()
  • reg_mindssc_gpu
  • reg_mind_gpu
  • reg_kld_gpu
  • reg_dti_gpu

Incomplete implementations:

  • Fix reg_spline_getDeformationField_gpu to accept composition
  • Fix reg_getImageGradient_gpu to accept interpolation and timepoints

@mmodat
Copy link
Contributor

mmodat commented Jul 14, 2023

@onurulgen The block matching test added above fails in Release., but runs fine in Debug. There might be a trouble with CPU or CUDA block matching. I'll add some unit tests for this as well (but unlikely this week).

@mmodat
Copy link
Contributor

mmodat commented Jul 26, 2023

Morning @onurulgen,

I was writing a test for the NMI gradient computation and found out that measures have to be initialised with a F3DContent. I think that NMI (and the other measures) should use the base Content instead. As we discussed last week, the measures computations do not need to know about the transformation model as all is based on a voxel-based approach. Would you be able to change this throughout? Happy to have a chat later today if this is not clear or you don't agree. Thanks

@onurulgen
Copy link
Member Author

We can't put into Content since it fills AladinContent with unnecessary stuff. I'll reorganise Content classes as

Content

onurulgen added a commit that referenced this issue Nov 15, 2023
- Optimise reg_getVoxelBasedNmiGradient_gpu()
- Get the function ready for multi-timepoint support
onurulgen added a commit that referenced this issue Nov 20, 2023
…92

 - Eliminate unnecessary Cuda::* functions
 - Refactor Cuda::CreateTextureObject()
onurulgen added a commit that referenced this issue Nov 24, 2023
onurulgen added a commit that referenced this issue Nov 24, 2023
onurulgen added a commit that referenced this issue Nov 28, 2023
onurulgen added a commit that referenced this issue Jan 9, 2024
onurulgen added a commit that referenced this issue Jan 10, 2024
onurulgen added a commit that referenced this issue Jan 30, 2024
@onurulgen onurulgen linked a pull request Aug 29, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: In Progress
Development

Successfully merging a pull request may close this issue.

3 participants