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

Implement masked Tucker #173

Merged
merged 9 commits into from
Jun 15, 2020
Merged

Implement masked Tucker #173

merged 9 commits into from
Jun 15, 2020

Conversation

aarmey
Copy link
Contributor

@aarmey aarmey commented Jun 11, 2020

I believe this implements masking for Tucker decomposition, which then fixes the last of issue #4. The test here is pretty minimal; if you have an idea for something better I can implement it.

@coveralls
Copy link

coveralls commented Jun 11, 2020

Coverage Status

Coverage decreased (-0.1%) to 90.424% when pulling 6c8ea1e on aarmey:masked-tucker into b479f18 on tensorly:master.

@JeanKossaifi
Copy link
Member

Thanks @aarmey, this is great!

For the test, we could add with a test case with a non uniform mask. For instance, perhaps we could build a low-rank Tucker tensor, mask a few values (e.g. from redundant components only) and check that they are correctly recovered?

@aarmey
Copy link
Contributor Author

aarmey commented Jun 13, 2020

Good suggestion—it should be implemented here. The test helped to reveal that masking really doesn't work with SVD initialization because the starting point reflects the masked value. I've added a warning about this in both Tucker and PARAFAC.

@JeanKossaifi
Copy link
Member

Thanks @aarmey, this is great!
Merging.

@JeanKossaifi JeanKossaifi merged commit ea0acd8 into tensorly:master Jun 15, 2020
@aarmey aarmey deleted the masked-tucker branch July 1, 2020 13:37
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

Successfully merging this pull request may close these issues.

None yet

3 participants