search

TomographicImaging / CCPi-Regularisation-Toolkit

The set of CPU/GPU optimised regularisation modules for iterative image reconstruction and other image processing tasks
Apache License 2.0
49 stars 25 forks source link

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

Closed paskino closed 3 months ago

paskino commented 3 months ago

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: https://github.com/TomographicImaging/CCPi-Regularisation-Toolkit/blob/71f8d304d804b54d378f0ed05539f01aaaf13758/src/Python/src/cpu_regularisers.pyx#L139-L141

and passed inverted:

https://github.com/TomographicImaging/CCPi-Regularisation-Toolkit/blob/71f8d304d804b54d378f0ed05539f01aaaf13758/src/Python/src/cpu_regularisers.pyx#L149-L155

24.0.0

The dimensions are created in the same order as in version 22, but used in the opposite order: https://github.com/TomographicImaging/CCPi-Regularisation-Toolkit/blob/b0e188875b7d1ad5cdf9fcde9c7a8005a0973384/src/Python/ccpi/filters/TV.py#L82-L88

dkazanc commented 3 months ago

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 commented 3 months ago

Tests are very useful, please add them.

paskino commented 3 months ago

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

dkazanc commented 3 months ago

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

casperdcl commented 3 months ago

released as v24.0.1