Skip to content

ENH: Add frozen dimensions to gufunc signatures #5015

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

Closed
wants to merge 1 commit into from

Conversation

jaimefrio
Copy link
Member

No description provided.

@jaimefrio
Copy link
Member Author

This adds the possibility of including numbers in the signature of a gufunc. These dimensions then become frozen at that value, and passing in an array with a different sized dimension will raise an error.

To demonstrate it, a cross1d gufunc has been added to numpy.core.umath_tests that performs cross products on 3D vectors:

In [1]: import numpy as np

In [2]: from numpy.core.umath_tests import cross1d

In [3]: cross1d.signature
Out[3]: '(3),(3)->(3)'

In [4]: a = np.random.rand(1000, 3)

In [5]: b = np.random.rand(1000, 3)

In [6]: np.allclose(np.cross(a, b), cross1d(a, b))
Out[6]: True

In [7]: %timeit np.cross(a, b)
10000 loops, best of 3: 76.1 us per loop

In [8]: %timeit cross1d(a, b)
100000 loops, best of 3: 13.1 us per loop

In [9]: c = np.random.rand(1000, 2)

In [10]: d = np.random.rand(1000, 2)

In [11]: cross1d(c, d)
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-11-72c66212e40c> in <module>()
----> 1 cross1d(c, d)

ValueError: cross1d: Operand 0 has a mismatch in its core dimension 0, with gufunc signature (3),(3)->(3) (size 2 is different from 3)

Needs some tests.

@njsmith
Copy link
Member

njsmith commented Aug 29, 2014

Needs to wait a bit to give the mailing list time to chime in.

Needs docs.

Needs tests, as you say. In particular in addition to tests for cross, I'd like to see a test case that defines a gufunc with multiple frozen dimensions that aren't all equal to each other.

@njsmith
Copy link
Member

njsmith commented Aug 29, 2014

Also you have universal test failures on Travis.

@jaimefrio jaimefrio force-pushed the gufuncs_frozen_dims branch from 0ee8860 to 58fef9b Compare August 29, 2014 04:43
@jaimefrio jaimefrio force-pushed the gufuncs_frozen_dims branch from 58fef9b to 64267ce Compare August 29, 2014 07:32
@jaimefrio
Copy link
Member Author

Man that was tough to track down! PyUFuncObjects are initialized both in ufunc_object.c, in function PyUFunc_FromFuncAndDataAndSignature, and in umathmodule.c, in function ufunc_frompyfunc. I was only setting the member pointer I added to PyUFuncObject to NULL in the first, but wasn't aware of the second, so calls to np.frompyfunc where eventually freeing an unitialized pointer and crashing everything.

Travis seem to be happier now...

@jaimefrio
Copy link
Member Author

Aside from the fact that it finally seems to be working without crashes or leaks, I fully agree on taking things slowly, and coming up with a well designed solution. Even if this is a small-ish, self-contained change, there is no rush.

@juliantaylor
Copy link
Contributor

hm this breaks ABI and C-API by extending the ufunc structure again (users might be doing the same from_pyfunc does) , though I want to do that too in another change.
Maybe we should first figure out a way to extend this structure more savely or put in a pointer to an internal ufunc object we can change as we like.
The former could be done by using the check_return or a hidden the iterator flag to indicate extension behind one of the malloc'd members.

@jaimefrio
Copy link
Member Author

I will eventually want another function pointer, for computed output dimensions, so I am +1 on anything that allows changing things with more freedom. Hiding my array of npy_intps, and my function pointer, in another malloc'ed array after its intended content is a little too hackish for my liking, as I think it will eventually turn the code into a nightmare to maintain.

My first thought after figuring out my from_pyfunc woes was that all those initializations belonged in a common ufunc_init function. So I wouldn't mind seeing PyUfuncObject get a handful of accessor functions in the API, and direct member access explicitly discouraged, somewhat like NpyIter does. Not sure how much stuff would be broken elsewhere if we did that.

@mrocklin
Copy link
Contributor

This appears to be stalled. Was there ever a follow on issue, PR, or further discussion that I might peruse? This sort of thing has been requested in dask.array and I'm thinking through how best to handle it in a way that might be future proof to further numpy developments.

@eric-wieser
Copy link
Member

We might also need this to implement #8720, since sometimes the output is defined to be size 0

@mattip
Copy link
Member

mattip commented Feb 13, 2019

This was superseded by the NEP-20 PRs, thanks for the idea which in the end was implemented!

@mattip mattip closed this Feb 13, 2019
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.

7 participants