CCPi-Regularisation-Toolkit icon indicating copy to clipboard operation
CCPi-Regularisation-Toolkit copied to clipboard

ctypes: pass the dimension parameters in the correct order

Open paskino opened this issue 1 year ago • 2 comments

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.

  • closes #204

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

paskino avatar May 13 '24 09:05 paskino

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?

dkazanc avatar May 13 '24 10:05 dkazanc

Tests are very useful, please add them.

paskino avatar May 13 '24 14:05 paskino

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

paskino avatar Jun 03 '24 12:06 paskino

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

dkazanc avatar Jun 04 '24 09:06 dkazanc

released as v24.0.1

casperdcl avatar Jun 06 '24 09:06 casperdcl