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
Adds tests for PR 5659 #6664
Adds tests for PR 5659 #6664
Conversation
Also fixes some test to use the more descriptive assertEqual instead of assertTrue
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needed to be done. Thanks for doing it. Looks like it is running with no errors.
numba/tests/test_parfors.py
Outdated
src = cycle(gen()) | ||
return [next(src) for i in range(ct)] | ||
|
||
def gen_linespace_variants(self, ct): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you mean linespace or linspace?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oops, it's linspace.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for writing these extra tests @sklam, looks good, just one dead comment to remove and then can be merged.
numba/parfors/parfor.py
Outdated
@@ -1920,7 +1920,8 @@ def run(self, blocks): | |||
if isinstance(instr, ir.Assign): | |||
expr = instr.value | |||
lhs = instr.target | |||
if self._is_C_order(lhs.name): | |||
lhs_typ = self.pass_states.typemap[lhs.name] | |||
if self._is_C_or_F_order(lhs_typ): | |||
# only translate C order since we can't allocate F |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove this comment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this comment should go.
numba/parfors/parfor.py
Outdated
@@ -1920,7 +1920,8 @@ def run(self, blocks): | |||
if isinstance(instr, ir.Assign): | |||
expr = instr.value | |||
lhs = instr.target | |||
if self._is_C_order(lhs.name): | |||
lhs_typ = self.pass_states.typemap[lhs.name] | |||
if self._is_C_or_F_order(lhs_typ): | |||
# only translate C order since we can't allocate F |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this comment should go.
i've removed the dead comment |
thanks. |
There was a problem hiding this 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(es). Looks good, great to see this working.
Based on #5659
Adds tests for 1D, 2D, 3D and C/F order arrays for parfors.
Note for reviewer: the main changes to the PR starts in 2628ec4.
The only file I touched in the
test_parfors.py
.