-
-
Notifications
You must be signed in to change notification settings - Fork 85
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
Feature/412 add faer rs backend #549
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #549 +/- ##
=======================================
Coverage 92.11% 92.11%
=======================================
Files 177 177
Lines 23672 23672
=======================================
Hits 21806 21806
Misses 1866 1866 ☔ View full report in Codecov by Sentry. |
This is excellent, thank you! Rebasing on the current main should fix the failing CI. Could you add running the faer tests to the CI (section |
03c5907
to
146db74
Compare
done and done, fingers crossed this works now :) |
Hi, not meaning to rush you, just checking in to see whether there's something else I can do. |
@geo-ant - really great work here - THANKS! If I may ask, can you make an example, say from this lbfgs example? Thanks in advance. |
Hey @humphreylee, I'm definitely open to that but I'll wait until the maintainers get back to me with requests for changes so I can do it in one go. Right now I don't know what the status of this is. |
Hi @geo-ant , I'm very sorry for the long wait, I have difficulty finding the time nowadays. In general this looks pretty good to me, I just need to find some time to review this with the amount of attention that this PR deserves. I can't promise it, but I will try to do this this weekend. |
Hey @stefan-k, no need to apologise, none of us are getting paid for this. If you want, ping me and I can also hop on a call and take 20 min to guide you through the PR. The thing is pretty big but very formulaic and not nearly as complex as it might seem from the number of lines of code. Again, don't worry about being fast. I'll be happy to see this merged eventually, but life comes before open source :) |
Fixes: #412 add faer as math backend
This PR adds
faer
support to argmin-math.faer
is a fast and rapidly evolving matrix backend that is gaining popularity in the numeric Rust ecosystem. I'm not associated with the project.I've closely followed the logic of the
nalgebra
implementation and even manually copied and translated the tests, so that the behavior of thenalgebra
implementation is kept with thefaer
implementation. The one exception I made is that aneye_like
identity matrix can also be requested for non-square matrices without panicking. Otherwise I've kept the original behavior.All implementations are available on the owned
Mat<T>
type and many (but not all) are available on the non-owning reference-typeMatRef<T>
.