Skip to content

[MRG] Fix convolutional_barycenter2d kernel for non-symmetric images#132

Merged
rflamary merged 1 commit into
PythonOT:masterfrom
atong01:atong-fix-convolutional-barycenter
Mar 13, 2020
Merged

[MRG] Fix convolutional_barycenter2d kernel for non-symmetric images#132
rflamary merged 1 commit into
PythonOT:masterfrom
atong01:atong-fix-convolutional-barycenter

Conversation

@atong01

@atong01 atong01 commented Mar 5, 2020

Copy link
Copy Markdown
Contributor

Thank you for your awesome package!

The code and examples for ot.bregman.convolutional_barycenter2d assumes a symmetric image. This PR changes the kernel to allow for non-symmetric images and adds a simple test of this functionality.

This addresses Issue #124

@rflamary

rflamary commented Mar 6, 2020

Copy link
Copy Markdown
Collaborator

This is great, thank you for the PR.

I think one day we should implement a proper convolution instead of those matrix products (or even circular convolution with Fourier as an option).

you can add you name at the top of the file as Author . I will do the merge in the next days. I am still waiting for a few things but I will do a new release shortly.

@rflamary rflamary changed the title Fix convolutional_barycenter2d kernel for non-symmetric images [MRG] Fix convolutional_barycenter2d kernel for non-symmetric images Mar 6, 2020
@atong01 atong01 force-pushed the atong-fix-convolutional-barycenter branch from 863238f to d82e6eb Compare March 6, 2020 13:51
@atong01

atong01 commented Mar 6, 2020

Copy link
Copy Markdown
Contributor Author

Agreed a proper convolution does sound useful in some circumstances. Interesting to note a simple usage of scipy.ndimage.filter.gaussian_filter with this kernel is ~4X slower on the example images. I would guess matrix products are simply better optimized at this scale.

def K(x):
    # sigma to match previous behavior
    sigma = [np.sqrt(reg / 2) * dim for dim in x.shape]
    x = gaussian_filter(x, sigma=sigma, mode="constant")
    return x

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants