-
Notifications
You must be signed in to change notification settings - Fork 140
BUG: fixed IndexError in pwcca when number of neurons do not match #9
base: master
Are you sure you want to change the base?
Conversation
|
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here with What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
|
@arimorcos @rubai5 @yosinski can this be merged? |
@maxwell-aladago Does this affect the code or what it just a notational bug? |
It affected the code. |
|
@maxwell-aladago I am confused, if the if statement that you left unchanged, why does it matter which of the two layers we choose if they remove the small values anyway in their cca code? i.e. |
|
discussion of this here: moskomule/anatome#27 |
|
@maxwell-aladago I also noticed you didn't center the acts1, acts2 (nor did the original code)...isn't that a bug? the |
Given two activations
z_i \in \mathbb{R}^{n_i, T}andz_j \in \mathbb{R}^{n_j, T}wheren_j > n_i,pwcca(z_i, z_j)raises anIndexErrorwhereaspwcca(z_j, z_i)works fine.This pull request fixes that bug.