Nonlinear convolver implementation - A remark on CheckParameters()

Hi to all,

I am implementing a new Convolver module in castor that allows reconstructions
on clustered voxels. The implemented convolver is non-stationary (as I need to average values
of voxels in each cluster separately), therefore I overload some functions in vImageConvolver class.

Potentially, I need to change some things in vImageConvolver class itself when it does initialization.
For example, m_nbKernels, mp_dimKernelX, … become non-relevant in non-stationary case,
so a checker in CheckParameters() should not look at them if m_stationary=false. For example,
when my filter is non-stationary I define myself storage arrays and track their dimensions etc.,
but I recieve errors from superclass checker that some ‘unused’ members have not being allocated
or initialized properly.

I plan to change this code in the source code of castor and when the module will be ready push things
to GitLab. Just wanted to make a remark on this, it is not really a question.

I’ve made a PR request on this on gitlab.

Thanks for your contribution.

The name of the m_stationnary variable is not well chosen.

It’s purpose is to specify if the convolver transpose need to be different from the convolver. They can be equivalent even for a non stationnary situation as soon as the kernels for the different positions are symmetric.

The m_nbKernels, mp_dimKernerX, etc, are made on purpose for non stationary kernels, which means that you do need more that one kernel. Then, the idea is that the positionning, or what is done with the multiple kernels, is handled by some more coding and arrays.
That was the original idea, which may not be the ideal one, but at least it was made to be generic.

What you are coding is not a convolver, so indeed you do need your own storage arrays and you do not need the kernel structures.

So I would suggest to keep everything as is and to not change the abstract class because of a particular case.
However, perhaps we need to change the name of m_stationnary to something like m_symmetric.

I agree with you on most of the things but I think it is important to clarify what is a Convolver, in general.
For me, convolver in most generic form is if:

  • Value(voxel) = \integral K(voxel, voxel~) d(voxel~), where K(*, *) is a kernel which is defined for all
    voxels

  • If K(x,y) = K(x-y) - then, convolver is stationary (i.e., shift invariant)

What I implement is still a convolution (operation is linear - averaging inside a cluster), but just non-stationary one (kernels change with spacial position). In general, if convolution is non-stationary, then
generally number of kernels should be equal number of voxels (as for every voxel kernel is different).
I understand the meaning of multiple kernels inside a module if your aim is to use several convolutions in one ‘logical convolution’.

My problem was CheckParameters() which was blocking module initialization: as I don’t use standard class fields they are left uninitialized and CheckParameters returns an error (there it is a simple 'if (…unitialized) then return error_code). What I did is that I just changed the ‘if condiiton’ inside CheckParameters().