Skip to content

Gradient of the difference norm between a CP and dense tensor#294

Merged
JeanKossaifi merged 6 commits into
tensorly:mainfrom
aarmey:CP-lstsq-grad
Aug 16, 2021
Merged

Gradient of the difference norm between a CP and dense tensor#294
JeanKossaifi merged 6 commits into
tensorly:mainfrom
aarmey:CP-lstsq-grad

Conversation

@aarmey

@aarmey aarmey commented Jul 7, 2021

Copy link
Copy Markdown
Contributor

A few notes:

  • This does not take the gradient with respect to the weights.
  • This only solves for the l2 norm difference. Adding other norms, if useful, probably wouldn't be too much work.
  • In testing, I figured that comparing to numerical differencing would be slow, so I check that the negative gradient is a direction of descent with some small perturbation.
  • There might be faster ways to calculate this... this is just what I came across in a literature search.

@codecov

codecov Bot commented Jul 7, 2021 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #294 (d6a9ddb) into main (9a115d9) will decrease coverage by 0.09%.
The diff coverage is 92.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #294      +/-   ##
==========================================
- Coverage   89.33%   89.24%   -0.10%     
==========================================
  Files          91       91              
  Lines        4857     4881      +24     
==========================================
+ Hits         4339     4356      +17     
- Misses        518      525       +7
Impacted Files Coverage Δ
tensorly/cp_tensor.py 83.50% <81.81%> (-0.27%) ⬇️
tensorly/tests/test_cp_tensor.py 100.00% <100.00%> (ø)
tensorly/contrib/decomposition/_tt_cross.py 89.30% <0.00%> (-3.15%) ⬇️
tensorly/decomposition/_tucker.py 89.62% <0.00%> (+0.04%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9a115d9...d6a9ddb. Read the comment docs.

yngvem
yngvem reviewed Aug 12, 2021
View reviewed changes

@yngvem yngvem left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice addition! I just have one note — with this implementation, we can compute the loss essentially for free (0.5*tl.sum(error**2)), and you are often interested in getting both the loss and the gradient simultaneously (e.g. in a line-search). Maybe we should add an optional keyword-argument return_loss that we can enable to compute the loss too?

Comment thread tensorly/cp_tensor.py Outdated
@aarmey

aarmey commented Aug 12, 2021

Copy link
Copy Markdown
Contributor Author

Flott, takk @yngvem! Your suggestions should be incorporated here. I think I've also fixed the test for the lower accuracy backends. Those should pass now.

@JeanKossaifi

JeanKossaifi commented Aug 16, 2021

Copy link
Copy Markdown
Member

Looks good to me, thanks @aarmey @yngvem! Merging.

@JeanKossaifi JeanKossaifi merged commit 80b13c0 into tensorly:main Aug 16, 2021
@aarmey aarmey deleted the CP-lstsq-grad branch August 16, 2021 19:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@yngvem yngvem yngvem left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@aarmey @JeanKossaifi @yngvem