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

ctypes: pass the dimension parameters in the correct order #205

Merged
merged 4 commits into from
Jun 6, 2024

Conversation

paskino
Copy link
Contributor

@paskino paskino commented May 13, 2024

The dimension parameter in v24.0.0 is passed reversed to what it was in version v22.0.0, see details below for FGP_TV.

In this PR, we invert the dims list at creation so that the parameters match what was in version 22.

Details

v22.0.0

The dimensions dims are created as:

dims[0] = inputData.shape[0]
dims[1] = inputData.shape[1]
dims[2] = inputData.shape[2]

and passed inverted:

TV_FGP_CPU_main(&inputData[0,0,0], &outputData[0,0,0], &infovec[0], regularisation_parameter,
iterationsNumb,
tolerance_param,
methodTV,
nonneg,
dims[2], dims[1], dims[0])
return (outputData,infovec)

24.0.0

The dimensions are created in the same order as in version 22, but used in the opposite order:

dims = list(inputData.shape)
if inputData.ndim == 2:
dims.append(1)
# float TV_FGP_CPU_main(float *Input, float *Output, float *infovector, float lambdaPar,
# int iterationsNumb, float epsil, int methodTV, int nonneg, int dimX, int dimY, int dimZ);
cilreg.TV_FGP_CPU_main(in_p, out_p, infovector_p, lambdaPar, iterationsNumb, epsil, methodTV, nonneg, dims[0], dims[1], dims[2])

@paskino paskino requested review from dkazanc and casperdcl May 13, 2024 09:08
@dkazanc
Copy link
Collaborator

dkazanc commented May 13, 2024

Thanks @paskino, this does look right to me. Shall I add some tests (3D) here as well, so we're more certain that all of the methods do what they should?

@paskino
Copy link
Contributor Author

paskino commented May 13, 2024

Tests are very useful, please add them.

)
from ccpi.filters.utils import cilregcuda

gpu_modules_available = cilregcuda is not None
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may not be sufficient to run the GPU tests.

In CIL we also verify that a NVIDIA GPU is installed by calling nvidia-smi along these lines

#nvidia
try:
    subprocess.check_output('nvidia-smi')
    has_nvidia = True
except:
    has_nvidia = False

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure, will add that. thanks.

@paskino
Copy link
Contributor Author

paskino commented Jun 3, 2024

@dkazanc can we merge this and open an issue to add unit tests?

@dkazanc
Copy link
Collaborator

dkazanc commented Jun 4, 2024

@paskino Yes, we can. I'm struggling to find time, but I'll come back to it. thanks.

demos/demo_cpu_regularisers3D.py Outdated Show resolved Hide resolved
@casperdcl casperdcl merged commit 3c56d78 into master Jun 6, 2024
12 checks passed
@casperdcl casperdcl deleted the fix_3D_regularisers branch June 6, 2024 09:08
@casperdcl
Copy link
Member

released as v24.0.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Issue with 3D GPU regularisers in v. 24.0.0
3 participants