Address PR comments (version 2)

Author: Theofilos Manitaras

docs/tutorial.rst

@@ -824,7 +824,7 @@ The first occurrence will be used as the reference value of the ``perf`` perform
 In our example, the ``perf`` key will be resolved in the ``daint:gpu`` scope giving us the reference value.
 
 Reference values in ReFrame are specified as a three-tuple comprising the reference value and lower and upper thresholds.
-Thresholds are specified as decimal fractions of the reference value. The lower threshold must lie in the [-1,0] interval, whereas the upper threshold must be lie in the [0,inf] interval.
+Thresholds are specified as decimal fractions of the reference value. For nonnegative reference values, the lower threshold must lie in the [-1,0], whereas the upper threshold must lie in the [0,inf] interval.
 In our example, the reference value for this test on ``daint:gpu`` is 50 Gflop/s ±10%. Setting a threshold value to :class:`None` disables the threshold.
 
 Combining It All Together

reframe/utility/sanity.py

@@ -471,26 +471,40 @@ def assert_reference(val, ref, lower_thres=None, upper_thres=None, msg=None):
     :arg val: The value to check.
     :arg ref: The reference value.
     :arg lower_thres: The lower threshold value expressed as a negative decimal
-        fraction of the reference value.  Must be in [-1, 0]. If ``None``, no
-        lower thresholds is applied.
+        fraction of the reference value.  Must be in [-1, 0] for ref >= 0.0 and
+        in [-inf, 0] for ref < 0.0.
+        If ``None``, no lower thresholds is applied.
     :arg upper_thres: The upper threshold value expressed as a decimal fraction
-        of the reference value.  Must be in [0, inf]. If ``None``, no upper
-        thresholds is applied.
+        of the reference value. Must be in [0, inf] for ref >= 0.0 and
+        in [0, 1] for ref < 0.0.
+        If ``None``, no upper thresholds is applied.
     :returns: ``True`` on success.
     :raises reframe.core.exceptions.SanityError: if assertion fails or if the
         lower and upper thresholds do not have appropriate values.
     """
     if lower_thres is not None:
-        try:
-            evaluate(assert_bounded(lower_thres, -1, 0))
-        except SanityError:
-            raise SanityError('invalid low threshold value: %s' % lower_thres)
+        if ref >= 0.0:
+            try:
+                evaluate(assert_bounded(lower_thres, -1, 0))
+            except SanityError:
+                raise SanityError('invalid low threshold value: %s' % lower_thres)
+        else:
+            try:
+                evaluate(assert_bounded(lower_thres, None, 0))
+            except SanityError:
+                raise SanityError('invalid low threshold value: %s' % lower_thres)
 
     if upper_thres is not None:
-        try:
-            evaluate(assert_bounded(upper_thres, None, None))
-        except SanityError:
-            raise SanityError('invalid high threshold value: %s' % upper_thres)
+        if ref >= 0.0:
+            try:
+                evaluate(assert_bounded(upper_thres, 0, None))
+            except SanityError:
+                raise SanityError('invalid high threshold value: %s' % upper_thres)
+        else:
+            try:
+                evaluate(assert_bounded(upper_thres, 0, 1))
+            except SanityError:
+                raise SanityError('invalid high threshold value: %s' % upper_thres)
 
     def calc_bound(thres):
         if thres is None:

unittests/test_sanity_functions.py

@@ -342,7 +342,9 @@ def test_assert_reference(self):
 
         # Check upper threshold values greater than 1
         self.assertTrue(sn.assert_reference(30.0, 10.0, None, 3.0))
-        self.assertTrue(sn.assert_reference(-50.0, -20.0, None, 2.0))
+        self.assertTrue(sn.assert_reference(-50.0, -20.0, -2.0, 0.5))
+        self.assertTrue(sn.assert_reference(30.0, 10.0, None, 3.0))
+        self.assertTrue(sn.assert_reference(-50.0, -20.0, -2.0, 0.5))
 
         self.assertRaisesRegex(
             SanityError,
@@ -382,6 +384,16 @@ def test_assert_reference(self):
                                'invalid low threshold value: 1\.2',
                                evaluate, sn.assert_reference(0.9, 1, 1.2, 0.1))
 
+        # check invalid thresholds greater than 1
+        self.assertRaisesRegex(SanityError,
+                               'invalid low threshold value: -2\.0',
+                               evaluate, sn.assert_reference(0.9, 1, -2.0, 0.1))
+        self.assertRaisesRegex(SanityError,
+                               'invalid high threshold value: 1\.5',
+                               evaluate, sn.assert_reference(-1.5, -1, -0.5, 1.5))
+
+
+
     def _write_tempfile(self):
         ret = None
         with NamedTemporaryFile('wt', delete=False) as fp: