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/#410, UE cast to int in 'unsafe' mode #500

Merged
merged 2 commits into from
Oct 13, 2022

Conversation

Moritz-Alexander-Kern
Copy link
Member

This PR addresses Issue #410

The following error occurred in

hist = np.bincount(np.int64(n_exp))

TypeError: Cannot cast array data from dtype('int64') to dtype('int32') according to the rule 'safe'

This might be fixed by using np.astype(int) instead of np.in64() to cast to int.
np.astype allows 'unsafe' casting by default, see:
https://numpy.org/doc/stable/reference/generated/numpy.ndarray.astype.html

@Moritz-Alexander-Kern Moritz-Alexander-Kern added the bugfix Fix for an indentified bug. label Jun 30, 2022
@Moritz-Alexander-Kern Moritz-Alexander-Kern added this to the v0.11.2 milestone Jul 18, 2022
@Moritz-Alexander-Kern
Copy link
Member Author

The numpy documentation suggest the following for casting:

To convert the type of an array, use the .astype() method (preferred) or the type itself as a function. For example:

z.astype(float)                 
array([0.,  1.,  2.])

np.int8(z)
array([0, 1, 2], dtype=int8)

See: https://numpy.org/doc/stable/user/basics.types.html

I suggest to follow numpys preferred method, but I don't have a strong opinion on this.

@Moritz-Alexander-Kern
Copy link
Member Author

Hey @sanjayankur31 ,

This should fix in PR #410 , please let me know if this solves the issue.

@sanjayankur31
Copy link

Thanks, it's on my list, just not managed to get to it yet. Will hopefully find some time in the next couple of days.

@sanjayankur31
Copy link

Seems to fix the issue on arm, all tests pass with this set of changes:

https://koji.fedoraproject.org/koji/taskinfo?taskID=91845461

Fedora's also dropped armv7hl support from the next release (F37), so we shouldn't have armv7hl related issues in the future:

https://www.fedoraproject.org/wiki/Changes/RetireARMv7

@Moritz-Alexander-Kern
Copy link
Member Author

Thanks @sanjayankur31 for having a look.

Build report looks good: https://koji.fedoraproject.org/koji/taskinfo?taskID=91845461
arm runner

@mdenker mdenker self-requested a review October 13, 2022 12:16
@mdenker mdenker merged commit fe413ac into NeuralEnsemble:master Oct 13, 2022
@Moritz-Alexander-Kern Moritz-Alexander-Kern deleted the fix/ue_analysis_int_cast branch October 28, 2022 07:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix Fix for an indentified bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Test failure on armv7hl: UETestCase.test__UE_surrogate
3 participants