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

BF: Cast input to np.repeat into int32. #789

Merged
merged 4 commits into from Nov 30, 2015

Conversation

arokem
Copy link
Contributor

@arokem arokem commented Nov 26, 2015

@arokem
Copy link
Contributor Author

arokem commented Nov 26, 2015

This looks like it could potentially be a bug in numpy. Thoughts from anyone?

@arokem
Copy link
Contributor Author

arokem commented Nov 28, 2015

Any thoughts about this one? @matthew-brett - does this look like a numpy bug to you? I believe it's the only remaining error on the bots at the moment

@matthew-brett
Copy link
Contributor

Yes, it does look like a bug in numpy, should be so for all 32-bit systems (I can replicate on 32-bit Linux) - see bug referenced above.

What about initializing points_per_line as 32-bit instead?

@matthew-brett
Copy link
Contributor

Sorry - better might be to do:

points_per_line = np.zeros([nb_lines], int)  # use native index type to avoid casting errors later

@seberg
Copy link

seberg commented Nov 29, 2015

Yeah. You can use int (or np.int_), but if you want to support 64bits on 64bit platforms (no idea if it matters for this one), you should make it np.intp. np.intp is the correct type for anything related to array sizes.

EDIT: intp maps to ssize_t/Py_SSize_t

@matthew-brett
Copy link
Contributor

Are int and intp different sizes on some platforms? I'd got used to them being the same, but maybe that isn't always true.

@seberg
Copy link

seberg commented Nov 29, 2015

No, int (the python one or np.int_) will map to long and on 64 bit windows long is unfortunatly 32bits.

@arokem
Copy link
Contributor Author

arokem commented Nov 29, 2015

OK - using np.intp now.

@arokem
Copy link
Contributor Author

arokem commented Nov 29, 2015

@@ -159,7 +159,7 @@ def lines_to_vtk_polydata(lines, colors=None):

# Get lines_array in vtk input format
lines_array = []
points_per_line = np.zeros([nb_lines], np.int64)
points_per_line = np.zeros([nb_lines], np.intp)
Copy link
Contributor

Choose a reason for hiding this comment

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

Could do with a comment here explaining why you're using intp.

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean why you are using intp instead of np.int64.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup. Linking back here in the comment.

On Sun, Nov 29, 2015 at 11:50 AM, Matthew Brett notifications@github.com
wrote:

In dipy/viz/utils.py
#789 (comment):

@@ -159,7 +159,7 @@ def lines_to_vtk_polydata(lines, colors=None):

 # Get lines_array in vtk input format
 lines_array = []
  • points_per_line = np.zeros([nb_lines], np.int64)
  • points_per_line = np.zeros([nb_lines], np.intp)

I mean why you are using intp instead of np.int64.


Reply to this email directly or view it on GitHub
https://github.com/nipy/dipy/pull/789/files#r46096969.

@arokem
Copy link
Contributor Author

arokem commented Nov 30, 2015

That seems to do the trick: http://nipy.bic.berkeley.edu/builders/dipy-py2.6-32/builds/654

Garyfallidis added a commit that referenced this pull request Nov 30, 2015
BF: Cast input to np.repeat into int32.
@Garyfallidis Garyfallidis merged commit b16c37e into dipy:master Nov 30, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants