Conversation
|
Also Fixes #557 |
| assert ak.all(vec.to_3D(z=1).z == 1) | ||
| assert ak.all(vec.to_3D(eta=1).eta == 1) | ||
| assert ak.all(vec.to_3D(theta=1).theta == 1) | ||
| assert_backend(ak.all(vec.to_3D(z=1).z == 1), True) |
There was a problem hiding this comment.
Wouldn't it be a lot clearer if these were just inside an if backend == "cpu" block? This really hides what's happening and that it's a simi-no-op? Though I guess it's still computing them, it's just not checking to see if it's true? Could it be equal to a backend == "cpu" constant or something?
There was a problem hiding this comment.
For the typetracer backend I want the expressions still be computed, but we can't compare it to numerical values, which is why the assertion is essentially a no-op. This is to make sure that the computation does not fail for the typetracer backend (without checking its result).
There was a problem hiding this comment.
We might want to compare the ak.forms.Form in the typetracer case, but that's kind of already checked generously in awkward-array itself - so not really useful.
So this is just to make sure a vector computation does not fail for the typetracer backend.
|
@Saransh-cpp could you please take a look at this - 1.6.0 doesn't really work for typetracers in a way that results in crashes. We may want to yank it. |
|
The release on PyPI can be yanked by @henryiii or @jpivarski. I can create a v1.6.1 release and add a note on the v1.6 release as soon as this PR is merged. |
|
Yanked 1.6.0. |
This PR fixes scikit-hep/coffea#1195 (the current vector release 1.6.0 is broken for typetracers)
The problem was that numpy (
lib) functions were directly called onTypeTracerArrays. Instead, we should've used the correspondingnplikeimplementation if it exists. This is only implemented for the typetracer backend as all other backends have already a dedicated dispatch mechanism.In order to ensure we're catching all errors with this I modified
tests/backend/test_awkward.pyto run every test also for the typetracer backend.If this PR looks good and gets merged we should make a new release (and maybe yank the previous one?).
Sorry for the inconvenience!
(cc @lgray)