Closes #4984: ak.array with negative numbers still has problems #4985
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1RyanK
force-pushed
the
4984-ak.array_with_negative_numbers_still_has_problems
branch
7 times, most recently
from
8b1ba0f to
3f7eb0f
Compare
1RyanK
added
the
blocking
1RyanK
force-pushed
the
4984-ak.array_with_negative_numbers_still_has_problems
branch
2 times, most recently
from
c65fa9e to
1ce06f9
Compare
1RyanK
force-pushed
the
4984-ak.array_with_negative_numbers_still_has_problems
branch
from
1ce06f9 to
a783f45
Compare
drculhane
approved these changes
Oct 17, 2025
Contributor
drculhane
left a comment
drculhane
left a comment
There was a problem hiding this comment.
I ran the unit tests, and also the one specific example you cited when you created the issue. Looks good. I concur that ak.array has become a bit of a mess, and glad to see that we now have an issue for that, too.
github-merge-queue bot
pushed a commit
that referenced
this pull request
Dec 4, 2025
…es (#5044) While PR #4985 technically fixed the issue of general negative bigint problems, it caused a performance regression. I believe this is primarily due to the overhead of creating the sign array. Instead, here's basically what happens in the code: ```python any_neg = np.any(flat < 0) req_bits: int if any_neg: req_bits = max(flat.max().bit_length(), (-flat.min()).bit_length()) + 1 else: req_bits = flat.max().bit_length() ``` Then the code figures out how many times to pull off `uint64` limbs, rather than waiting until everything is zero (which doesn't happen in the negative case, it just continues to be -1). The code runs out to ~~two~~ three separate Chapel functions depending on the case. If the input is just an `int64` or `uint64` array, it converts it directly to bigint (similar for `float64` or some other kind of floating point input). If the input is numpy's version of a bigint array, it goes to the multi-limb version (unfortunately, this is the case even if everything comes out to just one limb, but I think the performance loss here is not that bad). If the input data had any negative values, it treats the limbs as signed (all bits are positive and the top bit is negative). However, it's hard to create a bigint like this (AFAIK). So Chapel-side, it creates a signs array, strips off the top bit of every limb and treats it as a bool to reference later. Either way, it goes to the "Horner fold" step, which, as ChatGPT tells me, is possibly a faster way to create the bigints. Previously the code was bit shifting the limbs into the right spot and then adding them to the bigint value. The idea here is that you start with the highest limb of the data, then you bitshift it and add in the next limb, bitshift what you have and add in the next limb, and so on. You can read more about the generic version of this [here](https://en.wikipedia.org/wiki/Horner%27s_method) (take x = `2**64`). Then it adds in the signed bit as necessary. It also handles the case where `max_bits` is not -1. Hopefully any loss in performance is made up by a few factors: 1. Previously the Python code was stripping the limbs off by modding by `2**64` and then integer dividing by the same value. I think it could speed the code up to do a bitmask by `2**64 - 1` and then bitshift by 64. 2. Supposedly the Horner fold has better performance. 3. If the input is only one limb of `int64` or `uint64` data, it should go to the single limb version and that should run quicker. ~~This also handles all cases of numeric data to bigint output in a single function, so if the performance is back up, then I can cut out some code in the array function.~~ I went ahead and cut the old bigint code out. Closes #5043: Investigate performance loss from negative bigint changes
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I mainly was trying to fix issues surrounding things like
ak.array([-2**200]), but while I was messing around inak.arrayI noticed a few other things that needed fixing.This is needed for #4593.
Summary of Changes
strcheck to raise a clearTypeErrorfor scalar string inputs.np.ndarrayandpd.Seriesintact.bigintinference forobjectarrays containing only integers.Purpose:
Fixes incorrect bigint conversion for large negative numbers and cleans up input normalization and dtype inference logic.
Closes #4984: ak.array with negative numbers still has problems