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 parfor reduction issue with Python 3.12. #9334

Merged
merged 10 commits into from Dec 8, 2023
Merged

Conversation

DrTodd13
Copy link
Collaborator

@DrTodd13 DrTodd13 commented Dec 5, 2023

Resolves #9290.

Reorder reduction nodes for Python 3.12 to get back to the expected order for prior versions.

Copy link
Contributor

@stuartarchibald stuartarchibald left a comment

Choose a reason for hiding this comment

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

Thanks for the patch @DrTodd13. I've tested this and can see the issue in the Numba IR as per the description in the PR. There's a few typos to look at, but most of concern is the redefinition of nodes in the loop that's iterating over nodes (see inline comment). Thanks again for fixing this.

docs/upcoming_changes/9334.bug_fix.rst Outdated Show resolved Hide resolved
docs/upcoming_changes/9334.bug_fix.rst Outdated Show resolved Hide resolved
numba/parfors/parfor.py Outdated Show resolved Hide resolved
numba/parfors/parfor.py Outdated Show resolved Hide resolved
numba/parfors/parfor.py Outdated Show resolved Hide resolved
numba/parfors/parfor.py Outdated Show resolved Hide resolved
numba/parfors/parfor.py Outdated Show resolved Hide resolved
numba/parfors/parfor.py Outdated Show resolved Hide resolved
Comment on lines +3884 to +3885
# With Python 3.12 changes, Numba may separate that assignment
# to a new basic block. The code below looks and sees if an
Copy link
Contributor

Choose a reason for hiding this comment

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

I've checked the IR and this is indeed the case, the PreLowerStripPhis pass introduces this form.

@stuartarchibald stuartarchibald added 4 - Waiting on author Waiting for author to respond to review and removed 3 - Ready for Review labels Dec 6, 2023
DrTodd13 and others added 8 commits December 6, 2023 08:10
Co-authored-by: stuartarchibald <stuartarchibald@users.noreply.github.com>
Co-authored-by: stuartarchibald <stuartarchibald@users.noreply.github.com>
Co-authored-by: stuartarchibald <stuartarchibald@users.noreply.github.com>
Co-authored-by: stuartarchibald <stuartarchibald@users.noreply.github.com>
Co-authored-by: stuartarchibald <stuartarchibald@users.noreply.github.com>
Co-authored-by: stuartarchibald <stuartarchibald@users.noreply.github.com>
Co-authored-by: stuartarchibald <stuartarchibald@users.noreply.github.com>
Co-authored-by: stuartarchibald <stuartarchibald@users.noreply.github.com>
@stuartarchibald stuartarchibald added 4 - Waiting on reviewer Waiting for reviewer to respond to author and removed 4 - Waiting on author Waiting for author to respond to review labels Dec 6, 2023
Copy link
Contributor

@stuartarchibald stuartarchibald left a comment

Choose a reason for hiding this comment

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

Thanks for the patch!

@stuartarchibald stuartarchibald added 5 - Ready to merge Review and testing done, is ready to merge and removed 4 - Waiting on reviewer Waiting for reviewer to respond to author labels Dec 8, 2023
@sklam sklam merged commit 6548265 into numba:main Dec 8, 2023
21 checks passed
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Python 3.12] [Task] Parfor test failures due to Python 3.12 bytecode changes.
3 participants