Skip to content
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

Fix #7713: Ensure _prng_random_hash return has correct bitwidth #7748

Merged
merged 1 commit into from
Jan 17, 2022

Conversation

gmarkall
Copy link
Member

When the hash width is 32 bits, get_next_int32() needs to be used because that returns a 32-bit integer, as opposed to get_next_int(), which returns a 64-bit integer regardless of the supplied bitwidth.

This commit also alters the signature of _prng_random_hash() to use _Py_hash_t as the return type - it will be equal to types.intp, but the intent should be clearer.

Tested so far on Python 3.10 on 32- and 64-bit Windows with:

python -m numba.runtests numba.tests.test_closure.TestObjmodeFallback.test_issue2955 numba.tests.test_dictobject.TestDictObject.test_issue6570_alignment_padding numba.tests.test_hashing.TestNumberHashing.test_complex numba.tests.test_hashing.TestNumberHashing.test_floats numba.tests.test_hashing.TestTupleHashing.test_heterogeneous_tuples numba.tests.test_hashing.TestNumberHashing.test_py310_nan_hash numba.tests.test_hashing.TestTupleHashing.test_homogeneous_tuples numba.tests.test_sets.TestFloatSets.test_constructor numba.tests.test_sets.TestFloatSets.test_iterator numba.tests.test_sets.TestUnboxing.test_numbers numba.tests.test_sets.TestSetReflection.test_reflect_clean numba.tests.test_sets.TestSetReflection.test_reflect_conditional numba.tests.test_sets.TestSetReflection.test_reflect_simple numba.tests.test_sets.TestExamples.test_type_coercion_from_update numba.tests.test_sets.TestUnboxing.test_tuples numba.tests.test_sets.TestFloatSets.test_update

This is the list of 16 failing tests reported in #7713 (comment)

Fixes #7713.

When the hash width is 32 bits, get_next_int32 needs to be used because
that returns a 32-bit integer, as opposed to get_next_int, which returns
a 64-bit integer regardless of the supplied bitwidth.

This commit also alters the signature of _prng_random_hash to use
_Py_hash_t as the return type - it will be equal to types.intp, but the
intent should be clearer.
@esc
Copy link
Member

esc commented Jan 15, 2022

Giving this a full build farm run over the weekend to see if it fixes #7713 on our CI too!

Build numba_yaml_148 has started

@esc esc added the BuildFarm Passed For PRs that have been through the buildfarm and passed label Jan 15, 2022
@esc
Copy link
Member

esc commented Jan 15, 2022

Build farm green for win32 on e6df66d

@gmarkall
Copy link
Member Author

@esc Thanks for the BF run - I've now marked this as ready for review (Iooks like I forgot to do that after the CI passed on Friday night).

Copy link
Member

@esc esc left a comment

Choose a reason for hiding this comment

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

I checked for typos, read the description and ran this on the BF. It looks good. On the basis of what I have observed, I am marking this as ready to merge!

@esc esc added 5 - Ready to merge Review and testing done, is ready to merge and removed 3 - Ready for Review labels Jan 17, 2022
@sklam sklam merged commit 0e2e32d into numba:master Jan 17, 2022
esc pushed a commit to esc/numba that referenced this pull request Jan 26, 2022
Fix numba#7713: Ensure _prng_random_hash return has correct bitwidth
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5 - Ready to merge Review and testing done, is ready to merge BuildFarm Passed For PRs that have been through the buildfarm and passed Effort - short Short size effort needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tests using float hashing fail on win32+Py3.10 for 0.55rc1
3 participants