Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Complex product should be used in eltwise product #3

Closed
vadimkantorov opened this issue Mar 5, 2018 · 6 comments
Closed

Complex product should be used in eltwise product #3

vadimkantorov opened this issue Mar 5, 2018 · 6 comments

Comments

@vadimkantorov
Copy link

vadimkantorov commented Mar 5, 2018

I was investigating differences in results of this package and of the original Caffe version. Things found so far:

  1. Rfft can be used directly, it is provided by pytorch_fft
  2. Complex product should be used here: https://github.com/DeepInsight-PCALab/CompactBilinearPooling-Pytorch/blob/master/CompactBilinearPooling.py#L91, the tf.multiply does complex product, original caffe version does complex product
  3. Output should be mutliplied by output_dim in order to achieve full equivalence with original caffe version
@dichen-cd
Copy link
Collaborator

Many thanks for your advice! I will investigate into that later!

@dichen-cd
Copy link
Collaborator

After some doc rushing I think tf.multiply does element-wise product instead of complex product. The comment at L141 also indicates that. And if this is true, element-wise on the real part and imag part respectively should be equivalent to element-wise product at the two parts at once.

@vadimkantorov
Copy link
Author

from my tests, it's the complex product that matches the original Caffe version

very roughly based on your code, i made a version that uses new PyTorch fft support: https://gist.github.com/vadimkantorov/d9b56f9b85f1f4ce59ffecf893a1581a

@dichen-cd
Copy link
Collaborator

Awesome!

I'll check the caffe version later.

@vadimkantorov
Copy link
Author

vadimkantorov commented Apr 19, 2018

a link to original author's caffe impl (complex product): https://github.com/gy20073/compact_bilinear_pooling/blob/master/caffe-20160312/src/caffe/layers/compact_bilinear_layer.cpp#L219

if tf.multiply doesn't do complex product, it may be a bug in the tf reimpl

@dichen-cd
Copy link
Collaborator

Checked the original paper again. Although the paper doesn't make an explicit statement, I believe u r right. It makes more sense for element-wise complex product than naive production.

Many thanks for your help! I'll update this repo soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants