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

Projection on SPDs is not projecting onto SPDs #3

Closed
kellertuer opened this issue Apr 15, 2021 · 2 comments
Closed

Projection on SPDs is not projecting onto SPDs #3

kellertuer opened this issue Apr 15, 2021 · 2 comments

Comments

@kellertuer
Copy link

kellertuer commented Apr 15, 2021

Hi, nice to see another package doing optimizationon manifolds! I have not yet had the time to check this versus what pymanopt is doing (I think they use tensor flow as a backend, too?) But I just noticed that

https://github.com/master/tensorflow-manopt/blob/93402f6770d5b3c45f232340fddfa92a7126f19a/tensorflow_manopt/manifolds/symmetric_positive.py#L37-L41

This might be wrong. For SPDs, the characteristic property is, that all eigenvalues are positive, so this projection is not projection onto the manifold (of SPDs) but onto the set of positive semidefinite matrices. There is no projection onto the SPDs since that set is open in the set of (symmetric) matrices.

@master
Copy link
Owner

master commented May 8, 2021

Hi - thank you for the feedback! tensorflow-manopt differs from pymanopt in two aspects: 1) it implements a different set of optimizers, focusing on (adaptive) SGD methods, and 2) tensorflow-manopt aims for interoperability with the TensorFlow API, rather than just using its autodiff capabilities. See, for example, Keras-compatible layers implementation.

I agree with your remark about SPD projection. It's implemented this way for backward compatibility reasons, so perhaps it should print a warning on calling.

@kellertuer
Copy link
Author

Thanks for the explanation, it seems reasonable to do an own project then.

yes, it should bring a warning I think.

@master master closed this as completed Feb 14, 2022
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