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

matrix shapes problem with entropic_fused_gromov_barycenters #574

Closed
youssef62 opened this issue Nov 11, 2023 · 3 comments
Closed

matrix shapes problem with entropic_fused_gromov_barycenters #574

youssef62 opened this issue Nov 11, 2023 · 3 comments

Comments

@youssef62
Copy link

youssef62 commented Nov 11, 2023

Describe the bug

When I call entropic_fused_gromov_barycenters , I have the following error :
ValueError: shapes (10,1) and (10,1) not aligned: 1 (dim 1) != 10 (dim 0)

Screenshots

image

Code sample

n = 10 

g1 = nx.erdos_renyi_graph(n,0.5)
g2  = nx.erdos_renyi_graph(n,0.5)

c1 = nx.adjacency_matrix(g1).toarray().astype(np.float64)
c2 = nx.adjacency_matrix(g2).toarray().astype(np.float64)

y = [[[1]]*n]*n
y = np.array(y).astype(np.float64) 

f , c  = fgw_barycenters(n,y,[c1,c2],alpha=0.5 )
f , c  = entropic_fused_gromov_barycenters(n,y,[c1,c2],alpha=0.5)   
print(c)
print(f)

Expected behavior

entropic_fused_gromov_barycenters should act like fgw_barycenters .

Environment (please complete the following information):

  • OS (e.g. MacOS, Windows, Linux): MacOS
  • Python version:
  • How was POT installed (source, pip, conda):pip

Output of the following code snippet:

import platform; print(platform.platform())
import sys; print("Python", sys.version)
import numpy; print("NumPy", numpy.__version__)
import scipy; print("SciPy", scipy.__version__)
import ot; print("POT", ot.__version__)

macOS-12.5.1-arm64-arm-64bit
Python 3.11.5 (main, Aug 24 2023, 15:09:32) [Clang 14.0.0 (clang-1400.0.29.202)]
NumPy 1.26.0
SciPy 1.11.3
POT 0.9.1

Additional context

I think the cause of the problem is in lines 986,987 in _bregman.py

Y = update_feature_matrix(lambdas, Ys_temp, T_temp, p)
Ms = [dist(Ys[s], Y) for s in range(len(Ys))]

dist takes entries in the form (ns,d) where update_feature_matrix returns ouputs of dimension (d,n). Either a transpose of Y or a change or the output type of update_feature_matrix would solve it.

I wanted to do a pr to fix but I could not run the test locally. Do you just make test ?
this is the error I encounter when I do that :
image

@cedricvincentcuaz
Copy link
Collaborator

cedricvincentcuaz commented Nov 11, 2023

Hello @youssef62 ,
thank you for your feedback. Indeed the transpose should be considered for the features. Tests were not helpful to spot this as we had n=d...
I also spotted other flaws in these functions, I will open an issue for this matter.

Then if you want to contribute to POT in the future: you'll need to make your install able to read .pyx file.
I think that running this command line in the POT folder should do the trick:
python3 setup.py build_ext --inplace

And then it's explained in the POT doc how to go for a PR. Including local tests, pep8 etc. If you still have doubts you can check at the command lines to run tests in an existing PR ;)

@youssef62
Copy link
Author

Thank you for the quick reply. I also noticed that after adding the transpose, there were no errors but the results were always matrices with the same number on all entries.
Otherwise , do you know why I have that message when I run the tests ? should I ask this question in a discussion ?

@cedricvincentcuaz
Copy link
Collaborator

I edited my previous message about your message error.
Then for your results, there might be many reasons that you should check to be sure that it is a bug and not just the intended behaviour:

  • epsilon plays a crucial role for entropic solvers. Note that using solver through **kwargs will use different entropic FGW solvers.
  • sensitivity to init_X and alpha.

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

No branches or pull requests

2 participants