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

Moves tf to the numpy interface #407

Merged
merged 5 commits into from Jul 8, 2022
Merged

Moves tf to the numpy interface #407

merged 5 commits into from Jul 8, 2022

Conversation

aarmey
Copy link
Contributor

@aarmey aarmey commented May 14, 2022

The tensorflow numpy interface now seems to work, so I'm interested in feedback on this. One consideration is this interface only came out a version or two ago, and so we might deprecate the old interface first before moving up the required version of tensorflow.

@codecov
Copy link

codecov bot commented May 14, 2022

Codecov Report

Merging #407 (9c2d6e2) into main (7460dd2) will decrease coverage by 0.04%.
The diff coverage is 100.00%.

❗ Current head 9c2d6e2 differs from pull request most recent head 725c42f. Consider uploading reports for the commit 725c42f to get more accurate results

@@            Coverage Diff             @@
##             main     #407      +/-   ##
==========================================
- Coverage   88.47%   88.42%   -0.05%     
==========================================
  Files         107      107              
  Lines        6367     6367              
==========================================
- Hits         5633     5630       -3     
- Misses        734      737       +3     
Impacted Files Coverage Δ
tensorly/decomposition/robust_decomposition.py 100.00% <100.00%> (ø)
tensorly/tenalg/proximal.py 67.21% <0.00%> (-0.48%) ⬇️
tensorly/decomposition/_cp.py 86.90% <0.00%> (-0.37%) ⬇️

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@JeanKossaifi
Copy link
Member

It looks good to me. The big question is whether we keep TensorFlow or just remove it altogether.
If the new interface works transparently perhaps it's worth keeping just in case some users still rely on it.

@aarmey
Copy link
Contributor Author

aarmey commented Jun 26, 2022

The new interface seems to work transparently for me. Perhaps we include it but reevaluate if there are future issues? (I know it has frequently been a headache.)

@JeanKossaifi
Copy link
Member

Great, I've merged the conflict. Let me know if I've missed anything, otherwise looks good to merge!

@aarmey aarmey merged commit bbe9568 into tensorly:main Jul 8, 2022
@aarmey aarmey deleted the numpy-tf branch July 8, 2022 18:17
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

2 participants