Implement dpnp.erf through pybind11 extension
#2551
Conversation
|
Array API standard conformance tests for dpnp=0.19.0dev3=py313h509198e_68 ran successfully. |
|
View rendered docs @ https://intelpython.github.io/dpnp/index.html |
antonwolfy
force-pushed
the
add-erf-impl
branch
from
c261a83 to
410dcb5
Compare
antonwolfy
requested review from
AlexanderKalistratov,
ndgrigorian,
vlad-perevezentsev and
vtavana
as code owners
There was a problem hiding this comment.
LGTM!
Thank you @antonwolfy for the implementation of dpnp.erf and significant cleanup of cython code!!!
antonwolfy
force-pushed
the
add-erf-impl
branch
from
a801c13 to
78640ba
Compare
… result, mismatching scipy
…th, and cron tests run Applly pre-commit rule in toml file
Applly pre-commit rule in toml file
…to pass dpnp with scipy>=1.16
Add a comment to casting in ErfFunctor implementation
antonwolfy
force-pushed
the
add-erf-impl
branch
from
cd3c5d5 to
2de4f06
Compare
|
@ndgrigorian, let me know if you have more comments |
| namespace dpnp::kernels::erf | ||
| { | ||
| template <typename argT, typename Tp> | ||
| struct ErfFunctor |
There was a problem hiding this comment.
to this day, I'm not sure why we didn't use inheritance with these structs from a common parent with sensible defaults. That may be the smart thing to do someday. For sure not in this PR though. Leaving this comment to remember.
| result = dpnp.special.erf(ia) | ||
| expected = scipy.special.erf(a) |
There was a problem hiding this comment.
it might be more readable and reliable to simply test against the expected values, i.e, that erf(nan) gives nan, erf(inf) gives 1, and erf(-inf) gives -1
this way bugs or changes in behavior in scipy don't impact this test
There was a problem hiding this comment.
also wouldn't mind tests for 0 and -0
There was a problem hiding this comment.
It's the current dpnp approach to test against numpy or scipy to ensure alignment in the behavior.
Btw, I've added a dedicated test for signed zero.
4 participants
The PR changes the implementation of
dpnp.erffunction. It now invokesoneapi::mkl::vmimplementation from pybind11 extension of OneMKL VM if possible or uses dedicated kernel from pybind11 extension ofufunc.