ENH(OPT,TST): make test_transformations run 7 sec instead of 150 or so #413
Conversation
| @@ -45,6 +62,7 @@ def test_rename(collection): | |||
|
|
|||
|
|
|||
| def test_product(collection): | |||
| collection = collection.clone() | |||
There was a problem hiding this comment.
oh -- leftovers -- will kill now
There was a problem hiding this comment.
and killed that entire commit for now anyways -- humanity is not ready for it ;)
yarikoptic
force-pushed
the
enh-test-transformations
branch
2 times, most recently
from
3458ec6 to
690f78d
Compare
|
Thanks, this is great! Per #320, I eventually want to move away entirely from using the bundled datasets for transformer testing, in favor of synthetic and much smaller data. That would speed things up further while making it much easier to evaluate against ground truth properly. But this PR makes that less urgent. :) |
oh - feel free to close without merging to keep the urge! ;) |
| types=['events'], | ||
| scan_length=SCAN_LENGTH, | ||
| merge=True, | ||
| sampling_rate=10, |
There was a problem hiding this comment.
btw, IIRC interesting "effects" did appear when I set it to 0.31, you get wonderful
_____________________________________________ test_resample_dense ______________________________________________
collection = <bids.variables.kollekshuns.BIDSRunVariableCollection object at 0x7fae82ce3810>
def test_resample_dense(collection):
new_sampling_rate = 50
old_sampling_rate = collection.sampling_rate
upsampling = float(new_sampling_rate) / old_sampling_rate
collection['RT'] = collection['RT'].to_dense(old_sampling_rate)
old_rt = collection['RT'].clone()
> collection.resample(new_sampling_rate, in_place=True)
bids/analysis/tests/test_transformations.py:192:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
bids/variables/kollekshuns.py:279: in resample
kind=kind)
bids/variables/variables.py:460: in resample
f = interp1d(x, self.values.values.ravel(), kind=kind)
/usr/lib/python2.7/dist-packages/scipy/interpolate/interpolate.py:433: in __init__
_Interpolator1D.__init__(self, x, y, axis=axis)
/usr/lib/python2.7/dist-packages/scipy/interpolate/polyint.py:60: in __init__
self._set_yi(yi, xi=xi, axis=axis)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
self = <scipy.interpolate.interpolate.interp1d object at 0x7fae81d4f050>
yi = array([0.000e+00, 1.793e+00, 1.637e+00, 0.000e+00, 0.000e+00, 0.000e+00,
...e+00, 0.000e+00,
1.440e+00, 0.000e+00, 2.071e+00, 0.000e+00, 0.000e+00])
xi = array([ 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12,
..., 880, 881, 882, 883,
884, 885, 886, 887, 888, 889, 890, 891, 892, 893])
axis = -1
def _set_yi(self, yi, xi=None, axis=None):
if axis is None:
axis = self._y_axis
if axis is None:
raise ValueError("no interpolation axis specified")
yi = np.asarray(yi)
shape = yi.shape
if shape == ():
shape = (1,)
if xi is not None and shape[axis] != len(xi):
> raise ValueError("x and y arrays must be equal in length along "
"interpolation axis.")
E ValueError: x and y arrays must be equal in length along interpolation axis.
There was a problem hiding this comment.
Ah, good catch. Maybe open a separate issue for that.
There was a problem hiding this comment.
@yarikoptic great minds fail alike
|
No no no—I'm perfectly happy being lazy and explaining my laziness by saying that your hard work robbed me of my motivation to improve things. :) |
|
forgot about this one . python 3.5 (nothing else) failed with I will merge master and see if it reproduces |
I thought also to decrease number of runs, but then test_split is not behaving, so decided to stay at this. Now tests here would run for 25-30 sec in contrast to 150-180 on my laptop
Creating a collection apparently is more expensive. Clone() is needed since some tests do modification inplace. If they did not it was possible to declare this fixture as (scope="module"). So we need to return a clone each time. This causes full run time of 9 sec vs 30 sec.
yarikoptic
force-pushed
the
enh-test-transformations
branch
from
a06773a to
9a1c357
Compare
Codecov Report
@@ Coverage Diff @@
## master #413 +/- ##
=======================================
Coverage 62.48% 62.48%
=======================================
Files 27 27
Lines 4569 4569
Branches 1176 1176
=======================================
Hits 2855 2855
Misses 1429 1429
Partials 285 285
Continue to review full report at Codecov.
|
|
Ok, seems to be green. Since it was blessed already, I will merge |
Also in principle, if tests take proper care about cloning where needed, then no caching would be needed, fixture could get
(scope="module")and get called only once - giving even more speed up. But ATM some test(s) do cause inplace modifications of the collection, and I didn't want to just blindly add.clone()everywhere, so added it once in the fixture. The whole test_transformations.py runs 7 sec for me now (instead of 150 originally), but I think even this is too much given that it really doesn't manipulate much of data ;)