Skip to content

MAINT: Move array-prep and type resolution to earlier#19257

Merged
mattip merged 2 commits intonumpy:mainfrom
seberg:main-ufunc-move-prep-and-resolution
Jun 21, 2021
Merged

MAINT: Move array-prep and type resolution to earlier#19257
mattip merged 2 commits intonumpy:mainfrom
seberg:main-ufunc-move-prep-and-resolution

Conversation

@seberg
Copy link
Copy Markdown
Member

@seberg seberg commented Jun 15, 2021

This also fixes a bug in the masked handling of array prep, that
was seems to have been simply completely broken.

Note that the long term goal is to unify the masked and non-masked,
but that is tricky right now due to the different signatures.


Based off (and includes) gh-19254. So merge that before reviewing this one.

/* Initialize all arrays (we usually only need a small part) */
memset(operands, 0, nop * sizeof(*operands));
memset(operation_descrs, 0, nop * sizeof(*operation_descrs));
memset(output_array_prepare, 0, nout * sizeof(*output_array_prepare));
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not NULL'ing everything seemed a small but noticable micro-optimization. This could probably be continued to use alloca (which would make this a single large memset).

Moves around wheremask which should be a ref-count fix for stranger error paths.

@seberg seberg force-pushed the main-ufunc-move-prep-and-resolution branch 2 times, most recently from 56d2743 to f897d10 Compare June 17, 2021 18:53
This also fixes a bug in the masked handling of array prep, that
was seems to  have been simply completely broken.

Note that the long term goal is to unify the masked and non-masked,
but that is tricky right now due to the different signatures.
@seberg seberg force-pushed the main-ufunc-move-prep-and-resolution branch from f897d10 to d60d607 Compare June 17, 2021 18:57
*/
op[i] = op_tmp;
Py_DECREF(op_tmp);
op[i+nin] = op_tmp;
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is a bit annoying, but just to note that the whole execute_fancy_ufunc_loop is deleted in the follow-up.

assert_raises(TypeError, self.matmul, np.int8(5), np.int8(5))
assert_raises(TypeError, self.matmul, np.void(b'abc'), np.void(b'abc'))
assert_raises(ValueError, self.matmul, np.arange(10), np.void(b'abc'))
assert_raises(TypeError, self.matmul, np.arange(10), np.void(b'abc'))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this user-facing change require a release note?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This just changes the order of errors, so no. If you do np.matmul(np.arange(10), np.void([b'abc'] * 10)) you already get the type error right now.

x = np.add(a, a, where=np.array(True))
return
else:
x = np.add(a, a)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a change in behavior?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well... It is a bug fix in a sense. Previously __array_prepare__ wasn't called, which is why it didn't fail with where (although we never tested where=).

Co-authored-by: Matti Picus <matti.picus@gmail.com>
@mattip mattip merged commit 31e232c into numpy:main Jun 21, 2021
@mattip
Copy link
Copy Markdown
Member

mattip commented Jun 21, 2021

Thanks @seberg

@seberg seberg deleted the main-ufunc-move-prep-and-resolution branch June 21, 2021 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants