TST: machinery for tests requiring large memory + lapack64 smoketest#15021
TST: machinery for tests requiring large memory + lapack64 smoketest#15021charris merged 3 commits intonumpy:masterfrom
Conversation
|
Azure failures seems spurious ("[NuGet] The operation has timed out" when installing mingw) |
|
Thanks Pauli. |
eric-wieser
left a comment
eric-wieser
left a comment
There was a problem hiding this comment.
Mostly looks good, one bug and some nits
| with open('/proc/meminfo', 'r') as f: | ||
| for line in f: | ||
| p = line.split() | ||
| info[p[0].strip(':').lower()] = float(p[1]) * 1e3 |
There was a problem hiding this comment.
| info[p[0].strip(':').lower()] = float(p[1]) * 1e3 | |
| info[p[0].strip(':').lower()] = int(p[1]) * 1024 |
Based on https://lore.kernel.org/patchwork/patch/666444/, this measurement is in KiB
| 'k': 1e3, 'm': 1e6, 'g': 1e9, 't': 1e12, | ||
| 'kb': 1e3, 'mb': 1e6, 'gb': 1e9, 'tb': 1e12} |
There was a problem hiding this comment.
nit: consider including kib, MiB, etc
|
|
||
| def _parse_size(size_str): | ||
| """Convert memory size strings ('12 GB' etc.) to float""" | ||
| suffixes = {'': 1.0, 'b': 1.0, |
There was a problem hiding this comment.
nit: Perhaps for brevity:
suffixes = {'': 1.0, 'k': 1e3, 'm': 1e6, 'g': 1e9, 't': 1e12}
suffixes.update({k + 'b': v for k, v in suffixes.items()}) # allow a trailing b
There was a problem hiding this comment.
After adding kib/et al, it seemed cleaner to not do this.
| mem_free = -1 | ||
| else: | ||
| msg = '{0} GB memory required, but {1} GB available'.format( | ||
| free_bytes/1e9, mem_free/1e9) |
There was a problem hiding this comment.
nit: could use _ArrayMemoryError._size_to_string here to format numbers as prefixed bytes.
There was a problem hiding this comment.
Didn't change this: corner-case message in test suite, and probably would have involved refactoring the utility function out from _ArrayMemoryError.
| env_var, exc)) | ||
|
|
||
| msg = ('{0} GB memory required, but environment variable ' | ||
| 'NPY_AVAILABLE_MEM={1} set'.format( |
There was a problem hiding this comment.
| 'NPY_AVAILABLE_MEM={1} set'.format( | |
| 'NPY_AVAILABLE_MEM="{1} GB" set'.format( |
There was a problem hiding this comment.
{1} is the string value of the env var here
| m = size_re.match(size_str.lower()) | ||
| if not m or m.group(2) not in suffixes: | ||
| raise ValueError("value {!r} not a valid size".format(size_str)) | ||
| return float(m.group(1)) * suffixes[m.group(2)] |
There was a problem hiding this comment.
nit: sizes are ints,
| return float(m.group(1)) * suffixes[m.group(2)] | |
| return int(float(m.group(1)) * suffixes[m.group(2)]) |
| import sys | ||
| import os | ||
| import re | ||
|
|
|
@charris, looks like you merged while I was reviewing. It's unfortunate github doesn't provide an easy way to collect the diffs above into a patch once it's merged. |
|
@eric-wieser The changes look small, just make a new PR. I was kinda counting on that anyway :) |
3 participants
Implement test suite machinery for (automatically) skipping tests that require large amounts of memory.
Also, add a companion test for gh-15012, which checks 64-bit LAPACK works as expected.
(I ended up adding
np.dotcheck instead of theeighcheck, because although theeighone seems to take less memory, the runtime seems to be > 20min so it's not suitable here.)