Skip to content

Commit

Permalink
malloc check and status check
Browse files Browse the repository at this point in the history
  • Loading branch information
@gfardell
gfardell committed Sep 13, 2024
1 parent f8b513b commit 70b19dc
Show file tree
Hide file tree
Showing 4 changed files with 99 additions and 31 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
- Bug fixes:
- `ImageData` removes dimensions of size 1 from the input array. This fixes an issue where single slice reconstructions from 3D data would fail due to shape mismatches (#1885)
- Make Binner accept accelerated=False (#1887)
- Added checks on memory allocations within `FiniteDifferenceLibrary.cpp` and verified the status of the return in `GradientOperator` (#1929)
- Changes that break backwards compatibility:
- CGLS will no longer automatically stop iterations once a default tolerance is reached. The option to pass `tolerance` will be deprecated to be replaced by `optimisation.utilities.callbacks` (#1892)

Expand Down
13 changes: 11 additions & 2 deletions Wrappers/Python/cil/optimisation/operators/GradientOperator.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,7 @@ def adjoint(self, x, out=None):
cilacc.openMPtest.restypes = ctypes.c_int32
cilacc.openMPtest.argtypes = [ctypes.c_int32]

cilacc.fdiff4D.restype = ctypes.c_int32
cilacc.fdiff4D.argtypes = [ctypes.POINTER(ctypes.c_float),
ctypes.POINTER(ctypes.c_float),
ctypes.POINTER(ctypes.c_float),
Expand All @@ -300,6 +301,7 @@ def adjoint(self, x, out=None):
ctypes.c_int32,
ctypes.c_int32]

cilacc.fdiff3D.restype = ctypes.c_int32
cilacc.fdiff3D.argtypes = [ctypes.POINTER(ctypes.c_float),
ctypes.POINTER(ctypes.c_float),
ctypes.POINTER(ctypes.c_float),
Expand All @@ -311,6 +313,7 @@ def adjoint(self, x, out=None):
ctypes.c_int32,
ctypes.c_int32]

cilacc.fdiff2D.restype = ctypes.c_int32
cilacc.fdiff2D.argtypes = [ctypes.POINTER(ctypes.c_float),
ctypes.POINTER(ctypes.c_float),
ctypes.POINTER(ctypes.c_float),
Expand Down Expand Up @@ -404,7 +407,10 @@ def direct(self, x, out=None):
arg1 = [Gradient_C.ndarray_as_c_pointer(ndout[i]) for i in range(len(ndout))]
arg2 = [el for el in self.domain_shape]
args = arg1 + arg2 + [self.bnd_cond, 1, self.num_threads]
self.fd(x_p, *args)
status = self.fd(x_p, *args)

if status != 0:
raise RuntimeError('Call to C gradient operator failed')

for i, el in enumerate(self.voxel_size_order):
if el != 1:
Expand Down Expand Up @@ -448,7 +454,10 @@ def adjoint(self, x, out=None):
arg2 = [el for el in self.domain_shape]
args = arg1 + arg2 + [self.bnd_cond, 0, self.num_threads]

self.fd(out_p, *args)
status = self.fd(out_p, *args)
if status != 0:
raise RuntimeError('Call to C gradient operator failed')

out.fill(ndout)

#reset input data
Expand Down
28 changes: 28 additions & 0 deletions Wrappers/Python/test/test_Gradient.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
# CIL Developers, listed at: https://github.com/TomographicImaging/CIL/blob/master/NOTICE.txt

import unittest
from unittest.mock import Mock, patch
import logging
import numpy
from cil.framework import ImageGeometry
Expand Down Expand Up @@ -454,3 +455,30 @@ def test_GradientOperator_complex_data(self):
# check dot_test
for sd in [5, 10, 15]:
self.assertTrue(LinearOperator.dot_test(Grad, seed=sd))


def test_GradientOperator_cpp_failure_direct(self):
# Simulate the failure by setting the status to non-zero
ig = ImageGeometry(voxel_num_x = 2, voxel_num_y = 3, voxel_num_z=4)
data = ig.allocate('random')

Grad = GradientOperator(ig, backend='c')

# with the call to status = self.fd(args) returning a non-zero value
Grad.operator.fd = Mock(return_value=-1)
with self.assertRaises(RuntimeError):
Grad.direct(data)


def test_GradientOperator_cpp_failure_adjoint(self):
# Simulate the failure by setting the status to non-zero
ig = ImageGeometry(voxel_num_x = 2, voxel_num_y = 3, voxel_num_z=4)
data = ig.allocate('random')

Grad = GradientOperator(ig, backend='c')
res_direct = Grad.direct(data)

# with the call to status = self.fd(args) returning a non-zero value
Grad.operator.fd = Mock(return_value=-1)
with self.assertRaises(RuntimeError):
Grad.adjoint(res_direct)
88 changes: 59 additions & 29 deletions src/Core/FiniteDifferenceLibrary.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -266,21 +266,34 @@ int fdiff_adjoint_neumann(float *outimagefull, const float *inimageXfull, const
//runs over full data in x, y, z. then corrects elements for bounday conditions and sums
const size_t volume = nx * ny * nz;

//assumes nx and ny > 1
const bool z_dim = nz > 1;

float *outimage = outimagefull;
const float *inimageX = inimageXfull;
const float *inimageY = inimageYfull;
const float *inimageZ = inimageZfull;

float *tempX = (float *)malloc(volume * sizeof(float));
if (tempX == nullptr)
{
fprintf(stderr, "Error: Memory allocation failed\n");
return -1;
}

float *tempY = (float *)malloc(volume * sizeof(float));
float *tempZ;
if (tempY == nullptr)
{
fprintf(stderr, "Error: Memory allocation failed\n");
return -1;
}

if (z_dim)
float *tempZ = nullptr;
if (nz > 1)
{
tempZ = (float *)malloc(volume * sizeof(float));
if (tempZ == nullptr)
{
fprintf(stderr, "Error: Memory allocation failed\n");
return -1;
}
}

long long c;
Expand Down Expand Up @@ -321,7 +334,7 @@ int fdiff_adjoint_neumann(float *outimagefull, const float *inimageXfull, const
}
}

if (z_dim)
if (tempZ != nullptr)
{
#pragma omp for
for (ind = nx * ny; ind < volume; ind++)
Expand Down Expand Up @@ -358,7 +371,7 @@ int fdiff_adjoint_neumann(float *outimagefull, const float *inimageXfull, const
free(tempX);
free(tempY);

if (z_dim)
if (tempZ != nullptr)
free(tempZ);

// //now the rest of the channels
Expand Down Expand Up @@ -391,21 +404,35 @@ int fdiff_adjoint_periodic(float *outimagefull, const float *inimageXfull, const
//runs over full data in x, y, z. then correctects elements for bounday conditions and sums
const size_t volume = nx * ny * nz;

//assumes nx and ny > 1
const bool z_dim = nz > 1;

float *outimage = outimagefull;
const float *inimageX = inimageXfull;
const float *inimageY = inimageYfull;
const float *inimageZ = inimageZfull;

float *tempX = (float *)malloc(volume * sizeof(float));
if (tempX == nullptr)
{
fprintf(stderr, "Error: Memory allocation failed\n");
return -1;
}

float *tempY = (float *)malloc(volume * sizeof(float));
float *tempZ;
if (tempY == nullptr)
{
fprintf(stderr, "Error: Memory allocation failed\n");
return -1;
}

float *tempZ = nullptr;

if (z_dim)
if (nz > 1)
{
tempZ = (float *)malloc(volume * sizeof(float));
if (tempZ == nullptr)
{
fprintf(stderr, "Error: Memory allocation failed\n");
return -1;
}
}

long long c;
Expand Down Expand Up @@ -445,7 +472,7 @@ int fdiff_adjoint_periodic(float *outimagefull, const float *inimageXfull, const
}
}

if (z_dim)
if (tempZ != nullptr)
{

#pragma omp for
Expand Down Expand Up @@ -483,7 +510,7 @@ int fdiff_adjoint_periodic(float *outimagefull, const float *inimageXfull, const
free(tempX);
free(tempY);

if (z_dim)
if (tempZ != nullptr)
free(tempZ);

//now the rest of the channels
Expand Down Expand Up @@ -516,67 +543,70 @@ DLL_EXPORT int fdiff4D(float *imagefull, float *gradCfull, float *gradZfull, flo
int nThreads_initial;
threads_setup(nThreads, &nThreads_initial);

int status;
if (boundary)
{
if (direction)
fdiff_direct_periodic(imagefull, gradXfull, gradYfull, gradZfull, gradCfull, nx, ny, nz, nc);
status = fdiff_direct_periodic(imagefull, gradXfull, gradYfull, gradZfull, gradCfull, nx, ny, nz, nc);
else
fdiff_adjoint_periodic(imagefull, gradXfull, gradYfull, gradZfull, gradCfull, nx, ny, nz, nc);
status = fdiff_adjoint_periodic(imagefull, gradXfull, gradYfull, gradZfull, gradCfull, nx, ny, nz, nc);
}
else
{
if (direction)
fdiff_direct_neumann(imagefull, gradXfull, gradYfull, gradZfull, gradCfull, nx, ny, nz, nc);
status = fdiff_direct_neumann(imagefull, gradXfull, gradYfull, gradZfull, gradCfull, nx, ny, nz, nc);
else
fdiff_adjoint_neumann(imagefull, gradXfull, gradYfull, gradZfull, gradCfull, nx, ny, nz, nc);
status = fdiff_adjoint_neumann(imagefull, gradXfull, gradYfull, gradZfull, gradCfull, nx, ny, nz, nc);
}

omp_set_num_threads(nThreads_initial);
return 0;
return status;
}
DLL_EXPORT int fdiff3D(float *imagefull, float *gradZfull, float *gradYfull, float *gradXfull, size_t nz, size_t ny, size_t nx, int boundary, int direction, int nThreads)
{
int nThreads_initial;
threads_setup(nThreads, &nThreads_initial);
int status;

if (boundary)
{
if (direction)
fdiff_direct_periodic(imagefull, gradXfull, gradYfull, gradZfull, NULL, nx, ny, nz, 1);
status = fdiff_direct_periodic(imagefull, gradXfull, gradYfull, gradZfull, NULL, nx, ny, nz, 1);
else
fdiff_adjoint_periodic(imagefull, gradXfull, gradYfull, gradZfull, NULL, nx, ny, nz, 1);
status = fdiff_adjoint_periodic(imagefull, gradXfull, gradYfull, gradZfull, NULL, nx, ny, nz, 1);
}
else
{
if (direction)
fdiff_direct_neumann(imagefull, gradXfull, gradYfull, gradZfull, NULL, nx, ny, nz, 1);
status = fdiff_direct_neumann(imagefull, gradXfull, gradYfull, gradZfull, NULL, nx, ny, nz, 1);
else
fdiff_adjoint_neumann(imagefull, gradXfull, gradYfull, gradZfull, NULL, nx, ny, nz, 1);
status = fdiff_adjoint_neumann(imagefull, gradXfull, gradYfull, gradZfull, NULL, nx, ny, nz, 1);
}

omp_set_num_threads(nThreads_initial);
return 0;
return status;
}
DLL_EXPORT int fdiff2D(float *imagefull, float *gradYfull, float *gradXfull, size_t ny, size_t nx, int boundary, int direction, int nThreads)
{
int nThreads_initial;
threads_setup(nThreads, &nThreads_initial);
int status;

if (boundary)
{
if (direction)
fdiff_direct_periodic(imagefull, gradXfull, gradYfull, NULL, NULL, nx, ny, 1, 1);
status = fdiff_direct_periodic(imagefull, gradXfull, gradYfull, NULL, NULL, nx, ny, 1, 1);
else
fdiff_adjoint_periodic(imagefull, gradXfull, gradYfull, NULL, NULL, nx, ny, 1, 1);
status = fdiff_adjoint_periodic(imagefull, gradXfull, gradYfull, NULL, NULL, nx, ny, 1, 1);
}
else
{
if (direction)
fdiff_direct_neumann(imagefull, gradXfull, gradYfull, NULL, NULL, nx, ny, 1, 1);
status = fdiff_direct_neumann(imagefull, gradXfull, gradYfull, NULL, NULL, nx, ny, 1, 1);
else
fdiff_adjoint_neumann(imagefull, gradXfull, gradYfull, NULL, NULL, nx, ny, 1, 1);
status = fdiff_adjoint_neumann(imagefull, gradXfull, gradYfull, NULL, NULL, nx, ny, 1, 1);
}

omp_set_num_threads(nThreads_initial);
return 0;
return status;
}

0 comments on commit 70b19dc

Please sign in to comment.