Skip to content

Commit

Permalink
Raise ValueError when blocks>frames in make balance (#138)
Browse files Browse the repository at this point in the history
* fix #137
* make balance raises ValueErro  if any slices would remain empty, ie if n_blocks > n_frames
* changelog

* disable warning

* test

* test fix
  • Loading branch information
yuxuanzhuang authored Aug 22, 2020
1 parent 9b3b566 commit 6f2c77e
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 28 deletions.
1 change: 1 addition & 0 deletions CHANGELOG
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ Fixes
* fixed RDF functions (gave wrong results if step != 1) (#114)
* fixed InterRDF_s function (gave wrong results if density=True) (#120)
* fixed Contact fails with uneven blocks. (#140)
* raise ValueError when n_blocks > n_frames (Issue #137, PR #138)

Changes
* requires MDAnalysis >= 1.0.0 (#122)
Expand Down
3 changes: 1 addition & 2 deletions pmda/test/test_parallel.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,8 @@ def test_scheduler(analysis, scheduler):
def test_nframes_less_nblocks_warning(analysis):
u = mda.Universe(analysis._top, analysis._traj)
n_frames = u.trajectory.n_frames
with pytest.warns(UserWarning):
with pytest.raises(ValueError):
analysis.run(stop=2, n_blocks=4, n_jobs=2)
assert len(analysis.res) == 2


@pytest.mark.parametrize('n_blocks', np.arange(1, 11))
Expand Down
45 changes: 21 additions & 24 deletions pmda/test/test_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,39 +61,35 @@ def _test_make_balanced_slices(n_blocks, start, stop, step, scale):
frames = traj_frames[start:stop:step]
n_frames = len(frames)

slices = make_balanced_slices(n_frames, n_blocks,
start=start, stop=stop, step=step)
if n_frames >= n_blocks:
slices = make_balanced_slices(n_frames, n_blocks,
start=start, stop=stop, step=step)

assert len(slices) == n_blocks
assert len(slices) == n_blocks

# assemble frames again by blocks and show that we have all
# the original frames; get the sizes of the blocks
# assemble frames again by blocks and show that we have all
# the original frames; get the sizes of the blocks

block_frames = []
block_sizes = []
for bslice in slices:
bframes = traj_frames[bslice]
block_frames.extend(list(bframes))
block_sizes.append(len(bframes))
block_sizes = np.array(block_sizes)
block_frames = []
block_sizes = []
for bslice in slices:
bframes = traj_frames[bslice]
block_frames.extend(list(bframes))
block_sizes.append(len(bframes))
block_sizes = np.array(block_sizes)

# check that we have all the frames accounted for
assert_equal(np.asarray(block_frames), np.asarray(frames))
# check that we have all the frames accounted for
assert_equal(np.asarray(block_frames), np.asarray(frames))

# check that the distribution is balanced
if n_frames >= n_blocks:
# check that the distribution is balanced
assert np.all(block_sizes > 0)
minsize = n_frames // n_blocks
assert len(np.setdiff1d(block_sizes, [minsize, minsize+1])) == 0, \
"For n_blocks <= n_frames, block sizes are not balanced"
else:
# pathological case; we will have blocks with length 0
# and n_blocks with 1 frame
zero_blocks = block_sizes == 0
assert np.sum(zero_blocks) == n_blocks - n_frames
assert np.sum(~zero_blocks) == n_frames
assert len(np.setdiff1d(block_sizes[~zero_blocks], [1])) == 0, \
"For n_blocks>n_frames, some blocks contain != 1 frame"
with pytest.raises(ValueError, match="n_blocks must be smaller"):
slices = make_balanced_slices(n_frames, n_blocks,
start=start, stop=stop, step=step)


@pytest.mark.parametrize('n_blocks', [1, 2, 3, 4, 5, 7, 10, 11])
Expand Down Expand Up @@ -125,7 +121,8 @@ def test_make_balanced_slices_empty(n_blocks, start, step):
(5, 4, -1, None, None), (0, 5, -1, None, None),
(5, 0, -1, None, None),
(5, 4, None, -1, None), (5, 4, 3, 2, None),
(5, 4, None, None, -1), (5, 4, None, None, 0)])
(5, 4, None, None, -1), (5, 4, None, None, 0),
(4, 5, None, None, None)])
def test_make_balanced_slices_ValueError(n_frames, n_blocks,
start, stop, step):
with pytest.raises(ValueError):
Expand Down
9 changes: 7 additions & 2 deletions pmda/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ def make_balanced_slices(n_frames, n_blocks, start=None, stop=None, step=None):
`start`, `stop, and `step` are not the defaults (left empty or
set to ``None``) they must be provided as parameters.
n_blocks : int
number of blocks (>0)
number of blocks (>0 and <n_frames)
start : int or None
The first index of the trajectory (default is ``None``, which
is interpreted as "first frame", i.e., 0).
Expand Down Expand Up @@ -138,8 +138,10 @@ def make_balanced_slices(n_frames, n_blocks, start=None, stop=None, step=None):
For a `step` > 1, we use ``m[i] *= step``. The last slice will
never go beyond the original `stop` if a value was provided.
.. versionadded:: 0.2.0
.. versionadded:: 0.2.0
.. versionchanged:: 0.3.0
raise ValueError when n_blocks is larger than n_frames
"""

start = int(start) if start is not None else 0
Expand All @@ -150,6 +152,9 @@ def make_balanced_slices(n_frames, n_blocks, start=None, stop=None, step=None):
raise ValueError("n_frames must be >= 0")
elif n_blocks < 1:
raise ValueError("n_blocks must be > 0")
elif n_frames != 0 and n_blocks > n_frames:
raise ValueError(f"n_blocks must be smaller than n_frames: "
f"{n_frames}")
elif start < 0:
raise ValueError("start must be >= 0 or None")
elif stop is not None and stop < start:
Expand Down

0 comments on commit 6f2c77e

Please sign in to comment.