I just noticed the following error: loading a very large file (such as a resting state image from the HCP dataset, e.g. with dimensions (91, 109, 91, 1200)) works fine with nibabel and python 2.7. The same command on python 3.5 fails with the following error:
---------------------------------------------------------------------------
OverflowError Traceback (most recent call last)
<ipython-input-5-6d7286f1eb52> in <module>()
----> 1 in_vol = in_img.get_data()
/usr/local/lib/python3.5/dist-packages/nibabel/spatialimages.py in get_data(self, caching)
580 if self._data_cache is not None:
581 return self._data_cache
--> 582 data = np.asanyarray(self._dataobj)
583 if caching == 'fill':
584 self._data_cache = data
/usr/local/lib/python3.5/dist-packages/numpy/core/numeric.py in asanyarray(a, dtype, order)
512
513 """
--> 514 return array(a, dtype, copy=False, order=order, subok=True)
515
516 def ascontiguousarray(a, dtype=None):
/usr/local/lib/python3.5/dist-packages/nibabel/arrayproxy.py in __array__(self)
142 def __array__(self):
143 # Read array and scale
--> 144 raw_data = self.get_unscaled()
145 return apply_read_scaling(raw_data, self._slope, self._inter)
146
/usr/local/lib/python3.5/dist-packages/nibabel/arrayproxy.py in get_unscaled(self)
137 offset=self._offset,
138 order=self.order,
--> 139 mmap=self._mmap)
140 return raw_data
141
/usr/local/lib/python3.5/dist-packages/nibabel/volumeutils.py in array_from_file(shape, in_dtype, infile, offset, order, mmap)
512 if hasattr(infile, 'readinto'):
513 data_bytes = bytearray(n_bytes)
--> 514 n_read = infile.readinto(data_bytes)
515 needs_copy = False
516 else:
/usr/lib/python3.5/gzip.py in read(self, size)
272 import errno
273 raise OSError(errno.EBADF, "read() on write-only GzipFile object")
--> 274 return self._buffer.read(size)
275
276 def read1(self, size=-1):
/usr/lib/python3.5/_compression.py in readinto(self, b)
66 def readinto(self, b):
67 with memoryview(b) as view, view.cast("B") as byte_view:
---> 68 data = self.read(len(byte_view))
69 byte_view[:len(data)] = data
70 return len(data)
/usr/lib/python3.5/gzip.py in read(self, size)
467 buf = self._fp.read(io.DEFAULT_BUFFER_SIZE)
468
--> 469 uncompress = self._decompressor.decompress(buf, size)
470 if self._decompressor.unconsumed_tail != b"":
471 self._fp.prepend(self._decompressor.unconsumed_tail)
OverflowError: Python int too large for C unsigned int
Happy to provide further info, not sure what is going on.
As the error message says, that will just overflow a 32-bit unsigned integer.
Would you mind putting a 1/0 or similar at line 512 of volumeutils.py in order to raise an error, then check the size of n_bytes when reading in Python 2.7? I guess it is the same?
I don't know why that would happen, but it will depend on how Python implements gzip decompression. My guess is Python changed the implementation at some point, and the newer implementation happens to use an unsigned 32-bit int.
Can you check other Python versions? Sorry to ask...
I guess buffering will be more efficient. But - I think we need to cover the case where the read returns fewer bytes than we thought. How about something like:
n_read = 0
max_read = 2 ** 32 -1 # Max for unsigned 32-bit integer
while (n_read > n_bytes):
n_wanted = min([n_bytes - n_read, max_read])
n_got = infile.readinto(data_bytes[n_read:n_read + n_wanted])
n_read += n_got
if n_got != n_wanted:
break
Sounds great. Since it's gzip-specific, we should probably put that in _gzip_open, in openers.py.
For testing, I think a unit test on array_from_file using a BytesIO object could be fastest--writing to disk sucks. If you can think of a way to do the Nifti1Image with BytesIO and avoid actually writing to disk, please let me know.
I see that it's possible to pass a file-like object to gzip.GzipFile, I guess it would be possible to use that to simulate a gzipped stream that is long, but we would still need a machine that could handle at least 4GB images in memory.
we would still need a machine that could handle at least 4GB images in memory.
Well, I guess it'll have to be 4GB + the size of the GZIP output in memory, vs. 4GB (just the image in memory) that seems to be unavoidable. So 6GB vs. 4GB... the major concern is the time.
As I'm running this code, I'm realizing that it's gzip itself that's slow. I'll decrease compression level in the test...
Sure thing. I want a test case to make sure things are working as expected before doing a PR, so ... it's useful, even if we disable the test by default.
Activity
effigies commented on Oct 20, 2015
What specific file from HCP did you use to produce this crash?
Also, can you try with Python 3.4? I don't believe nibabel is testing on 3.5, yet.
surchs commented on Oct 20, 2015
I used a resting state nifti file with all 4 sessions combined, hence the 1200 timepoints.
matthew-brett commented on Oct 20, 2015
Is it possible you have a 32-bit version of Python 3.5?
surchs commented on Oct 20, 2015
Pretty sure I got 64
matthew-brett commented on Oct 20, 2015
Can you try debug at the IPython prompt after the error, and find out what the value of size is in the traceback?
surchs commented on Oct 20, 2015
Sure
matthew-brett commented on Oct 20, 2015
As the error message says, that will just overflow a 32-bit unsigned integer.
Would you mind putting a 1/0 or similar at line 512 of volumeutils.py in order to raise an error, then check the size of n_bytes when reading in Python 2.7? I guess it is the same?
surchs commented on Oct 20, 2015
It's pretty close:
/usr/local/lib/python3.5/dist-packages/nibabel/volumeutils.py in array_from_file(shape, in_dtype, infile, offset, order, mmap) 509 return np.array([]) 510 # Read data from file --> 511 1/0 512 infile.seek(offset) 513 if hasattr(infile, 'readinto'): ZeroDivisionError: division by zero In [5]: debug > /usr/local/lib/python3.5/dist-packages/nibabel/volumeutils.py(511)array_from_file() 510 # Read data from file --> 511 1/0 512 infile.seek(offset) ipdb> n_bytes 4332619200 ipdb>matthew-brett commented on Oct 20, 2015
So, that should overflow a 32-bit unsigned int as well. I guess the difference is in the Python implementations.
surchs commented on Oct 20, 2015
I'm not sure I'm following. Why would there be a 32 bit constraint when running on a 64 bit python shell?
matthew-brett commented on Oct 20, 2015
I don't know why that would happen, but it will depend on how Python implements gzip decompression. My guess is Python changed the implementation at some point, and the newer implementation happens to use an unsigned 32-bit int.
Can you check other Python versions? Sorry to ask...
surchs commented on Oct 20, 2015
not easily, no. Sorry as well. I'll try on another machine. But this is seriously weird.
matthew-brett commented on Oct 20, 2015
Fair enough. I'll do my best to replicate on my laptop.
effigies commented on Nov 8, 2015
Any movement on this?
bcipolli commented on Nov 8, 2015
I will take a look at this.
18 remaining items
bcipolli commented on Nov 14, 2015
Ah I see, sorry. Yes, you get the same error.
bcipolli commented on Nov 14, 2015
Or, more succinctly:
n_read = 0 while n_read < n_bytes: to_read = (n_bytes - n_read) % (2**32 - 1) or (2**32 - 1) n_read += infile.readinto(data_bytes[n_read:n_read+to_read])matthew-brett commented on Nov 14, 2015
I guess another option is to disable use of readinto for Python 3.5...
bcipolli commented on Nov 14, 2015
Any preference?
matthew-brett commented on Nov 14, 2015
I guess buffering will be more efficient. But - I think we need to cover the case where the read returns fewer bytes than we thought. How about something like:
n_read = 0 max_read = 2 ** 32 -1 # Max for unsigned 32-bit integer while (n_read > n_bytes): n_wanted = min([n_bytes - n_read, max_read]) n_got = infile.readinto(data_bytes[n_read:n_read + n_wanted]) n_read += n_got if n_got != n_wanted: breakbcipolli commented on Nov 14, 2015
Sounds great. Since it's gzip-specific, we should probably put that in _gzip_open, in openers.py.
For testing, I think a unit test on array_from_file using a BytesIO object could be fastest--writing to disk sucks. If you can think of a way to do the Nifti1Image with BytesIO and avoid actually writing to disk, please let me know.
I'll try the unit test in the meantime.
bcipolli commented on Nov 14, 2015
Er.... we'll also have to wrap GzipFile in that case... but it seems best to keep things gzip-specific...
matthew-brett commented on Nov 14, 2015
I see that it's possible to pass a file-like object to gzip.GzipFile, I guess it would be possible to use that to simulate a gzipped stream that is long, but we would still need a machine that could handle at least 4GB images in memory.
bcipolli commented on Nov 14, 2015
Well, I guess it'll have to be 4GB + the size of the GZIP output in memory, vs. 4GB (just the image in memory) that seems to be unavoidable. So 6GB vs. 4GB... the major concern is the time.
As I'm running this code, I'm realizing that it's gzip itself that's slow. I'll decrease compression level in the test...
matthew-brett commented on Nov 14, 2015
I hate to say this now, but maybe we could do without a test here, on the basis that it's too time-consuming and memory intensive to test routinely.
Or have a separate test repo, for these kinds of very long / slow tests, that we trigger travis on / run on the buildbots from time to time.
bcipolli commented on Nov 14, 2015
Sure thing. I want a test case to make sure things are working as expected before doing a PR, so ... it's useful, even if we disable the test by default.
bcipolli commented on Nov 14, 2015
How about an environment variable SLOW_TESTS that runs slow tests when set; otherwise, they're skipped?
matthew-brett commented on Nov 14, 2015
Sure. Even something like NIPY_EXTRA_TESTS which has a comma separated list of types of extra tests such as slow,bigmem.
BF: Fix nipy#362 using buffered gzip read.