sdfunni / SingleOrigin

Atom-finding Python code
GNU General Public License v3.0
0 stars 0 forks source link

Concatination array size mismatch in fit_gaussians #4

Closed realitychemist closed 4 months ago

realitychemist commented 4 months ago

The concatenation on line 2198 of utils.fit_gaussians https://github.com/sdfunni/SingleOrigin/blob/043dc6ab19d8c897253ac9b375706cddae8c3dca/src/SingleOrigin/utils.py#L2198-L2202 raises the following error for some images when peak_grouping_filter is not None:

ValueError: all the input array dimensions except for the concatenation axis must match exactly, but along dimension 0, the array at index 0 has size 12 and the array at index 1 has size 11

The shape of the first array in this concatenate should always be ((params.shape[0]-1)/6, 6) or ((params.shape[0]-1)/4, 4) depending on if fits are circular or not. As far as I can tell this is working as expected.

The second array in the concatenation will always have shape (num_gauss, 1), where num_gauss is calculated on line 2152 https://github.com/sdfunni/SingleOrigin/blob/043dc6ab19d8c897253ac9b375706cddae8c3dca/src/SingleOrigin/utils.py#L2152

The documentation for this function says p0 should always be shape ((6*n)+1,): https://github.com/sdfunni/SingleOrigin/blob/043dc6ab19d8c897253ac9b375706cddae8c3dca/src/SingleOrigin/utils.py#L2122

For n >= 8, this leads to num_gauss being too small, hence the concatenation error. Here's a plot showing this behavior: `num_gauss` vs `n`

Replacing line 2152 with num_gauss = int(np.ceil((p0.shape[0]-1)/6)) should fix this issue, although I'm not sure if hard-coding in 6 will cause errors for circular fitting (when n_params = 4). It looks like it should be fine, but as an alternative the reshaped array could be pre-calculated before the concatenation (something like params_shaped = params[:-1].reshape((-1, n_params))) and then the np.ones array could just be of shape (params_shaped.shape[0], 1).