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

Add trig functions and constants #398

Merged
merged 13 commits into from Apr 12, 2022
Merged

Conversation

j6k4m8
Copy link
Contributor

@j6k4m8 j6k4m8 commented Apr 1, 2022

This fixes #395 and #394!

@codecov
Copy link

codecov bot commented Apr 1, 2022

Codecov Report

Merging #398 (3230454) into main (81301b2) will increase coverage by 0.27%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #398      +/-   ##
==========================================
+ Coverage   88.31%   88.58%   +0.27%     
==========================================
  Files         107      107              
  Lines        6238     6238              
==========================================
+ Hits         5509     5526      +17     
+ Misses        729      712      -17     
Impacted Files Coverage Δ
tensorly/__init__.py 100.00% <ø> (ø)
tensorly/decomposition/_cp.py 88.72% <0.00%> (+1.09%) ⬆️
tensorly/tenalg/proximal.py 70.99% <0.00%> (+3.30%) ⬆️

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

@cohenjer
Copy link
Contributor

cohenjer commented Apr 4, 2022

Looks good to me, thanks for adding these !
There seems to be a few issues however with some backends, thanks @caglayantuna for pointing this out.

@j6k4m8
Copy link
Contributor Author

j6k4m8 commented Apr 4, 2022

Indeed — it doesn't appear that the issues are with this new PR code, but I'm happy to wait to merge this as the other issues work out, if you prefer (and I will see if I can figure out an easy fix for any of them!)

Deep links to test failure lines:

Looks like there might be an import issue with tensorflow, working on that now!

@JeanKossaifi
Copy link
Member

it doesn't appear that the issues are with this new PR code

Yes, I'm fixing the JAX issue in the PR you mentioned and MXNet issue should be fixed whenever they switch to 2.0, we should use the nightly build until then.

@j6k4m8
Copy link
Contributor Author

j6k4m8 commented Apr 4, 2022

Awesome! In that case, I think this PR is ready for final code review and merging (i.e., all issues with the actual trig functions and constants are ironed out, I believe)

@JeanKossaifi
Copy link
Member

Looks good to me, thanks @j6k4m8, merging!

@JeanKossaifi JeanKossaifi merged commit eab4567 into tensorly:main Apr 12, 2022
@j6k4m8 j6k4m8 deleted the trig-fns branch April 12, 2022 23:40
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.

Numerical constants?
3 participants