ENH: increase NPY_MAXARGS to 256 #4840
Conversation
Allows larger numbers of arguments for nditer using functions. This can be helpful for complex boolean expressions used in pytables. Closes numpygh-4398
|
some functions use MAXARGS * MAXDIMS sized arrays so their stack usage increases by 8, but it should still be below 100kb which is probably still ok. |
|
this increases the size of the PyArrayMultiIterObject which is public, luckily the structure using this size is at the end of the object so it should be abi compatible |
|
+1 for master. For 1.9, how confident are we that this won't create new ABI breakage? If it's risky then we might want to wait until 1.10 rather than risk new breakage in beta2. |
|
originally we wanted to put that into 1.8.1 but deemed it to risky. Unfortunately we forgot to add it to master earlier. |
|
Biggest worry I would have is some code snipplet accidentally assuming MAXARGS == MAXDIMS |
|
@juliantaylor I'm going to put this in. Do you intend this for 1.9? If not the mention in the release notes should be omitted. |
|
if you ask me its either put it into 1.9 or don't do it at all. |
|
I'd rather have it in 1.10 than 1.9 at this point. The question is whether the iterator will get rewritten. IIRC, @mwiebe had some comments about this in the misty past. |
|
its not a large rewrite, just grepping for all occurences of MAXARGS and replacing it with a heap allocation, should be pretty simple |
|
That sounds like the best solution then. |
|
alright, I guess pandas worked around this a long time ago anyway |
|
can we use variable sized C arrays in numpy now? |
|
You mean VLAs? Those are a C99 feature and MSVC hates C99... On Thu, Jul 10, 2014 at 11:46 PM, Julian Taylor notifications@github.com
Nathaniel J. Smith |
|
seems msvc still doesn't have them, so heap memory it is ... |
|
this is how this would look like: https://github.com/juliantaylor/numpy/tree/no-max-args how about having no limitation if your compiler doesn't suck? that would be really simple to do, just replace NPY_MAXARGS with NPY_NOP_VAR(name) which evaluates to 32 when compiled with msvc in most places, then we just need to replace the > MAXARGS checks with some large value to prevent stack smashing. |
|
What does |
|
gcc never dropped vlas |
|
Would alloca or some tiny helper functions help here? Or even tmalloc, for
|
|
clang doesn't implement alloca, for good reason, its very dangerous to use and vla's have obsoleted most usecases of it. |
I remember using them back in the 80's or there abouts, and at some point, maybe after C89, the same code gave a compilation error. VLAs were certainly not part of any C spec at that point but as a newcomer from Fortran I appreciated them. |
|
But to as to the suggestion to make use of VLAs, that sounds like a clean way to do things. It looks like gcc, clang, and icc support them and msvc will be no worse than before. |
|
I guess I'm a little wary about whether supporting extra arguments but only
|
|
@njsmith My thinking is that common numpy distributions will use gcc or intel, and the wheels on pypi will also be built with gcc. I also suspect that msvc will support them sometime in the next couple of years, protests to the contrary, so the problem will eventually be solved by the passage of time. Vlas are, I think, cleaner than malloc for this use, and if the vlas allow for adjustable size on the fly that would be a plus when there are small numbers of dimensions. That said, I don't know exactly how Julian intends to use them and we could just put this PR in as an alternative. |
|
It doesn't matter if msvc adds a feature tomorrow, we're still stuck On Mon, Jul 14, 2014 at 5:51 AM, Charles Harris notifications@github.com
Nathaniel J. Smith |
|
2010 support we don't have to drop, it's already broken. But why might dropping MSVC be a good idea? I'm not a Windows user, but my impression is that MSVC is more popular than gcc on Windows. |
|
@rgommers msvc would work just fine, but the maximum number of arguments would be limited to the current 32. I think that would affect a small number of people, and they would have a fallback it they needed more arguments. |
|
yeah, I got that from the discussion. which is why I wondered who benefits from entirely dropping MSVC users (except us, because of slightly less maintenance work). |
|
@rgommers I don't understand what you mean by dropping msvc users. |
|
fwiw the branch I posted which would also work with msvc is already almost all you need for more nditer args missing is only more ufunc args but I don't think that is as important. But there is just a bug somewhere which is only triggered for large amounts of arguments, probably some typo in my attempt at avoiding too many mallocs. I haven't spent much time debugging it yet but if someone sees the issue let me know and I might finish it up. |
|
@charris I was responding to the comment right above from Nathaniel: (and typo, "users" --> "support") |
Allows larger numbers of arguments for nditer using functions. This can
be helpful for complex boolean expressions used in pytables.
Closes gh-4398