Skip to content

Commit

Permalink
RangeSet, NodeSet: fix bad handling of 0-padding in nD
Browse files Browse the repository at this point in the history
RangeSetND: keep 0-padding info on full-expand heuristic folding

Padding was lost when performing a full expand/fold in some cases for
performance purposes (i.e. number of vectors >> number of nD items).

Also fixed pads() to always return the max padding length of each axis
(not a bugfix, but it's safer and may help normalize mismatching
padding).

NodeSet: fix nsiter() for nD nodesets when 0-padding is used (fix clubak
in that case).

Closes #286.

Change-Id: I2c836cc54dde8ad2c6ecd5270e7e43a22e504a5c
  • Loading branch information
thiell committed Feb 13, 2016
1 parent 5357ebc commit 021b242
Show file tree
Hide file tree
Showing 6 changed files with 59 additions and 9 deletions.
5 changes: 5 additions & 0 deletions ChangeLog
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
2016-02-12 S. Thiell <sthiell@stanford.edu>

* RangeSet.py and NodeSet.py: fix bad 0-padding handling by RangeSetND or
NodeSet objects in nD (ticket #286).

2016-02-09 S. Thiell <sthiell@stanford.edu>

* NodeSet.py: fix parser issue when brackets were used with nodeset
Expand Down
8 changes: 4 additions & 4 deletions lib/ClusterShell/NodeSet.py
Original file line number Diff line number Diff line change
Expand Up @@ -217,14 +217,14 @@ def __iter__(self):

def nsiter(self):
"""Object-based NodeSet iterator on single nodes."""
for pat, ivec, pad, autostep in self._iter():
for pat, ivec, pads, autostep in self._iter():
nodeset = self.__class__()
if ivec is not None:
if len(ivec) == 1:
nodeset._add_new(pat, \
RangeSet.fromone(ivec[0], pad[0] or 0))
pad = pads[0] or 0
nodeset._add_new(pat, RangeSet.fromone(ivec[0], pad))
else:
nodeset._add_new(pat, RangeSetND([ivec], None, autostep))
nodeset._add_new(pat, RangeSetND([ivec], pads, autostep))
else:
nodeset._add_new(pat, None)
yield nodeset
Expand Down
11 changes: 6 additions & 5 deletions lib/ClusterShell/RangeSet.py
Original file line number Diff line number Diff line change
Expand Up @@ -942,10 +942,9 @@ def dim(self):

def pads(self):
"""Get a tuple of padding length info for each dimension."""
try:
return tuple(rg.padding for rg in self._veclist[0])
except IndexError:
return ()
# return a tuple of max padding length for each axis
pad_veclist = ((rg.padding for rg in vec) for vec in self._veclist)
return tuple(max(pads) for pads in zip(*pad_veclist))

def get_autostep(self):
"""Get autostep value (property)"""
Expand Down Expand Up @@ -1157,7 +1156,9 @@ def _fold_multivariate_expand(self):
# Simple heuristic that makes us faster
if len(self._veclist) * (len(self._veclist) - 1) / 2 > max_length * 10:
# *** nD full expand is preferred ***
self._veclist = [[RangeSet.fromone(i) for i in tvec] \
pads = self.pads()
self._veclist = [[RangeSet.fromone(i, pad=pads[axis])
for axis, i in enumerate(tvec)]
for tvec in set(self._iter())]
return

Expand Down
14 changes: 14 additions & 0 deletions tests/CLIClubakTest.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,12 @@ def test_000_noargs(self):
self._clubak_t([], "]o[o]: bar\n", outfmt % "]o[o]")
self._clubak_t([], "foo:\n", "---------------\nfoo\n---------------\n\n")
self._clubak_t([], "foo: \n", "---------------\nfoo\n---------------\n \n")
# nD
self._clubak_t([], "n1c1: bar\n", outfmt % "n1c1")
# Ticket #286
self._clubak_t([], "n1c01: bar\n", outfmt % "n1c01")
self._clubak_t([], "n01c01: bar\n", outfmt % "n01c01")
self._clubak_t([], "n001c01: bar\nn001c02: bar\n", outfmt % "n001c01" + outfmt % "n001c02")

def test_001_verbosity(self):
"""test clubak (-q/-v/-d)"""
Expand All @@ -58,13 +64,21 @@ def test_002_b(self):
self._clubak_t(["-b"], "]o[o]: bar\n", outfmt % "]o[o]")
self._clubak_t(["-b"], "foo:\n", "---------------\nfoo\n---------------\n\n")
self._clubak_t(["-b"], "foo: \n", "---------------\nfoo\n---------------\n \n")
# nD
self._clubak_t(["-b"], "n1c1: bar\n", outfmt % "n1c1")
# Ticket #286
self._clubak_t(["-b"], "n1c01: bar\n", outfmt % "n1c01")
self._clubak_t(["-b"], "n001c01: bar\n", outfmt % "n001c01")
self._clubak_t(["-b"], "n001c01: bar\nn001c02: bar\n", outfmt % "n001c[01-02] (2)")

def test_003_L(self):
"""test clubak (line mode -L)"""
self._clubak_t(["-L"], "foo: bar\n", "foo: bar\n")
self._clubak_t(["-L", "-S", ": "], "foo: bar\n", "foo: bar\n")
self._clubak_t(["-bL"], "foo: bar\n", "foo: bar\n")
self._clubak_t(["-bL", "-S", ": "], "foo: bar\n", "foo: bar\n")
# nD
self._clubak_t(["-bL", "-S", ": "], "n1c01: bar\n", "n1c01: bar\n")

def test_004_N(self):
"""test clubak (no header -N)"""
Expand Down
18 changes: 18 additions & 0 deletions tests/NodeSetTest.py
Original file line number Diff line number Diff line change
Expand Up @@ -1422,6 +1422,13 @@ def test_nsiter(self):
'roma58-ipmi', 'roma59-ipmi',
'roma60-ipmi', 'roma61-ipmi', 'tigrou3'])
self.assertEqual(list(ns1), [str(ns) for ns in ns1.nsiter()])
# Ticket #286 - broken nsiter() in 1.7 with nD + 0-padding
ns1 = NodeSet("n0c01")
self.assertEqual(list(ns1), ['n0c01'])
self.assertEqual(list(ns1), [str(ns) for ns in ns1.nsiter()])
ns1 = NodeSet("n0c01,n1c01")
self.assertEqual(list(ns1), ['n0c01', 'n1c01'])
self.assertEqual(list(ns1), [str(ns) for ns in ns1.nsiter()])

def test_contiguous(self):
"""test NodeSet.contiguous() iterator"""
Expand Down Expand Up @@ -2520,3 +2527,14 @@ def test_unicode(self):
self._assertNode(nodeset, "node1")
# not sure about that, can it work if PYTHONIOENCODING is set?
self.assertRaises(UnicodeEncodeError, NodeSet, u"\u0ad0[000-042]")

def test_nd_fold_padding(self):
"""test NodeSet nD heuristic folding with padding"""
# Ticket #286 - not broken in 1.7
n1 = NodeSet("n1c01,n1c02,n1c03,n1c04,n1c05,n1c06,n1c07,n1c08,n1c09,n2c01,n2c02,n2c03,n2c04,n2c05,n2c06,n2c07,n2c08,n2c09,n3c01,n3c02,n3c03,n3c04,n3c05,n3c06,n3c07,n3c08,n3c09,n4c01,n4c02,n4c03,n4c04,n4c05,n4c06,n4c07")
self.assertEqual(str(n1), "n[1-3]c[01-09],n4c[01-07]")
self.assertEqual(len(n1), 34)
# Ticket #286 - broken in 1.7 - trigger RangeSetND._fold_multivariate_expand full expand
n1 = NodeSet("n1c01,n1c02,n1c03,n1c04,n1c05,n1c06,n1c07,n1c08,n1c09,n2c01,n2c02,n2c03,n2c04,n2c05,n2c06,n2c07,n2c08,n2c09,n3c01,n3c02,n3c03,n3c04,n3c05,n3c06,n3c07,n3c08,n3c09,n4c01,n4c02,n4c03,n4c04,n4c05,n4c06,n4c07,n4c08,n4c09")
self.assertEqual(str(n1), "n[1-4]c[01-09]")
self.assertEqual(len(n1), 36)
12 changes: 12 additions & 0 deletions tests/RangeSetNDTest.py
Original file line number Diff line number Diff line change
Expand Up @@ -459,6 +459,18 @@ def test_pads(self):
self.assertEqual(str(rn1), "02-06; 006-009,411\n01-02; 003-004\n")
self.assertEqual(len(rn1), 29)
self.assertEqual(rn1.pads(), (2, 3))
# Note: padding mismatch is NOT supported by ClusterShell
# We just track any regressions here (MAY CHANGE!)
rn1 = RangeSetND([['01-02', '003'], ['01-02', '0101'], ['02-06', '006-009,411']])
# here 0101 padding is changed to 101
self.assertEqual(str(rn1), '02-06; 006-009,411\n01-02; 003,101\n')
self.assertEqual(len(rn1), 29)
self.assertEqual(rn1.pads(), (2, 3))
rn1 = RangeSetND([['01-02', '0003'], ['01-02', '004'], ['02-06', '006-009,411']])
# here 004 padding is changed to 0004
self.assertEqual(str(rn1), '02-06; 006-009,411\n01-02; 0003-0004\n')
self.assertEqual(len(rn1), 29)
self.assertEqual(rn1.pads(), (2, 4)) # pads() returns max padding length by axis

def test_mutability_1(self):
rs0 = RangeSet("2-5")
Expand Down

0 comments on commit 021b242

Please sign in to comment.