From 9e93ce565d6e15e028c397643413892c390ee442 Mon Sep 17 00:00:00 2001 From: Andreas Sandberg Date: Wed, 20 Jan 2021 12:19:26 +0000 Subject: [PATCH] python: Require a unit in anyToFrequency and anyToLatency The anytToFrequency and anyToLatency conversion functions are currently ambiguous when called without a unit. Fix this by always requiring a unit. Change-Id: I5ea94e655f7ca82c0efe70b9f9f7f734fbf711c1 Signed-off-by: Andreas Sandberg Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/39435 Tested-by: kokoro Reviewed-by: Daniel Carvalho --- src/python/m5/util/convert.py | 54 ++++++++++++++++++------------- tests/pyunit/util/test_convert.py | 14 ++------ 2 files changed, 33 insertions(+), 35 deletions(-) diff --git a/src/python/m5/util/convert.py b/src/python/m5/util/convert.py index 1d78f82d5..e66eb5c04 100644 --- a/src/python/m5/util/convert.py +++ b/src/python/m5/util/convert.py @@ -199,32 +199,40 @@ def toLatency(value): return toMetricFloat(value, 'latency', 's') def anyToLatency(value): - """result is a clock period""" - try: - return 1 / toFrequency(value) - except (ValueError, ZeroDivisionError): - pass + """Convert a magnitude and unit to a clock period.""" - try: - return toLatency(value) - except ValueError: - pass - - raise ValueError("cannot convert '%s' to clock period" % value) + magnitude, unit = toNum(value, + target_type='latency', + units=('Hz', 's'), + prefixes=metric_prefixes, + converter=float) + if unit == 's': + return magnitude + elif unit == 'Hz': + try: + return 1.0 / magnitude + except ZeroDivisionError: + raise ValueError(f"cannot convert '{value}' to clock period") + else: + raise ValueError(f"'{value}' needs a valid unit to be unambiguous.") def anyToFrequency(value): - """result is a clock period""" - try: - return toFrequency(value) - except ValueError: - pass - - try: - return 1 / toLatency(value) - except ValueError as ZeroDivisionError: - pass - - raise ValueError("cannot convert '%s' to clock period" % value) + """Convert a magnitude and unit to a clock frequency.""" + + magnitude, unit = toNum(value, + target_type='frequency', + units=('Hz', 's'), + prefixes=metric_prefixes, + converter=float) + if unit == 'Hz': + return magnitude + elif unit == 's': + try: + return 1.0 / magnitude + except ZeroDivisionError: + raise ValueError(f"cannot convert '{value}' to frequency") + else: + raise ValueError(f"'{value}' needs a valid unit to be unambiguous.") def toNetworkBandwidth(value): return toMetricFloat(value, 'network bandwidth', 'bps') diff --git a/tests/pyunit/util/test_convert.py b/tests/pyunit/util/test_convert.py index a9c9d4642..da618436b 100644 --- a/tests/pyunit/util/test_convert.py +++ b/tests/pyunit/util/test_convert.py @@ -163,28 +163,18 @@ class ConvertTestSuite(unittest.TestCase): self.assertEqual(conv('1kHz'), 1e-3) self.assertRaises(ValueError, conv, '42k') - - @unittest.expectedFailure - def test_anyToLatency_ambiguous(self): - # This the behavior of anyToFrequency is currently ambiguous - # (and surprising) for unitless quantities. The following - # should be true to be consistent with the other conversion - # functions, but that isn't currently the case. - self.assertEqual(convert.anyToLatency('42'), 42.0) - + self.assertRaises(ValueError, conv, '42') def test_anyToFrequency(self): conv = convert.anyToFrequency - # This is ambiguous and should probably not be allowed. - self.assertEqual(conv('42'), 42.0) - self.assertEqual(conv('42kHz'), 42e3) self.assertEqual(conv('0.1s'), 10.0) self.assertEqual(conv('1ms'), 1000.0) self.assertRaises(ValueError, conv, '42k') + self.assertRaises(ValueError, conv, '42') def test_toNetworkBandwidth(self): conv = convert.toNetworkBandwidth -- 2.30.2