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 regression on gufunc serialization #6826

Merged
merged 3 commits into from Mar 23, 2021
Merged

Conversation

sklam
Copy link
Member

@sklam sklam commented Mar 15, 2021

Fixes #6821.

@sklam sklam added this to the 0.53.1 milestone Mar 15, 2021
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, couple of comments to address else looks good.

Comment on lines +33 to +40
dct = dict(
py_func=gb.py_func,
signature=gb.signature,
identity=self._identity,
cache=gb.cache,
is_dynamic=self._is_dynamic,
targetoptions=gb.targetoptions,
typesigs=gb._sigs,
Copy link
Contributor

Choose a reason for hiding this comment

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

Think self._frozen should be in here, it is transmittable state and should prevent further compilation.

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

got = np.zeros_like(arr)
double(arr, out=expect)
cloned(arr, out=got)
self.assertPreciseEqual(expect, got)
Copy link
Contributor

Choose a reason for hiding this comment

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

Wonder if these tests ought to also check, self._frozen, self.gufunc_builder._sigs, self._is_dynamic and self._identity, just to make sure the state is carrying appropriately?

Copy link
Member Author

Choose a reason for hiding this comment

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

These attributes are now tested explicitly

@stuartarchibald stuartarchibald added 4 - Waiting on author Waiting for author to respond to review Effort - short Short size effort needed and removed 3 - Ready for Review labels Mar 22, 2021
@sklam sklam 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 Mar 22, 2021
# expected value of attributes
self.assertTrue(cloned._frozen)

cloned.disable_compile()
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this has any effect but is harmless.

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 and fixes, looks good.

@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 Mar 23, 2021
@sklam sklam merged commit 056bf6b into numba:master Mar 23, 2021
@sklam sklam deleted the fix/iss6821 branch March 23, 2021 22:55
esc pushed a commit to esc/numba that referenced this pull request Mar 25, 2021
Fix regression on gufunc serialization

(cherry picked from commit 056bf6b)
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 Effort - short Short size effort needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Regression: guvectorized functions can no longer be pickled in numba 0.53
2 participants