Repository: reframe-hpc/reframe

Issue:
Performance threshold goes to -inf when it should be zero.
In attempting to create a performance test where zero is the correct value, I created the following reference (since a value of zero results in no reference check being performed; see https://github.com/reframe-hpc/reframe/issues/2857):

```
        self.reference = {
            '*': {
                'Gflops': (None, None, None, 'Gflops'),
                'Exponent': (None, None, None, '10exp'),
                'Time': (None, None, None, 'seconds'),
                'failed_tests': (.1, -1.0, 0, 'tests'),
                'skipped_tests': (.1, -1.0, 0, 'tests')
        }}
```
I was thinking for the failed and skipped tests, this would create a lower bound of zero and upper bound of .1, allowing zero to pass, but integers larger than that would fail.
This works for me in that values of zero pass, and 1 or greater values fail, but what is unexpected is that the lower bound gets set to -inf:

```
  * Reason: performance error: failed to meet references: skipped_tests=1 tests, expected 0.1 (l=-inf, u=0.1)
```
As an additional point of information when I set the lower bound values to -2.0 (or anything greater than 1 abs. value, to see what would happen), the following was printed:
```
	invalid low threshold value: -2.0
	invalid low threshold value: -2.0
```
But when I choose 2.0 as an upper bound, that seems to be fine:
```
                'failed_tests': (.1, -1.0, 2.0, 'tests'),
                'skipped_tests': (.1, -1.0, 2.0, 'tests')
```
results in the range being interpretted as follows:
```
  * Reason: performance error: failed to meet references: skipped_tests=1 tests, expected 0.1 (l=-inf, u=0.30000000000000004)
```
It seems to me that the treatment of lower and upper threshold values should be consistent, and this does not seem consistent.
Although when I switch the reference value to a negative, that reverses the issue:
```
                'failed_tests': (-1.0, 0, 2.0, 'tests'),
                'skipped_tests': (-1.0, 0, 2.0, 'tests')
```
results in:
```
	invalid high threshold value: 2.0
	invalid high threshold value: 2.0
```
and to confirm, I set the lower threshold to -2.0 with a negative reference value to check:
```
                'failed_tests': (-1.0, -2.0, 1.0, 'tests'),
                'skipped_tests': (-1.0, -2.0, 1.0, 'tests')
```
results in:
```
PASSED
```
Meaning there was no error with the -2.0 reference value in this case. I am not sure what value the upper bound was getting set to in this case, since it wasn't printed out, but since there was no error, it must not be getting set to zero. If it matches the pattern, I am thinking it might be set to inf.
I am thinking there is something odd that happens when the threshold range crosses zero or has one end that is zero.

Discussion:
@vkarak: Related to #2857.@vkarak: > This works for me in that values of zero pass, and 1 or greater values fail, but what is unexpected is that the lower bound gets set to -inf:

Indeed this is a bug. These checks should check against `None`:

https://github.com/reframe-hpc/reframe/blob/b7d86b1d777bc3bed7a3d71d95130376ab5d2820/reframe/utility/sanity.py#L579-L580

As for the rest of the behaviour that you describe, this is documented: http://localhost:8000/deferrable_functions_reference.html#reframe.utility.sanity.assert_reference

The assumption made is that the extracted quantity is either positive or negative so the bounds should conform to this. That's why for example, you can't have a lower threshold > 1 for positive references, because this would imply a negative value for positive quantity.

Solution patch:
diff --git a/reframe/utility/sanity.py b/reframe/utility/sanity.py
index a4f57a301..1228586ff 100644
--- a/reframe/utility/sanity.py
+++ b/reframe/utility/sanity.py
@@ -8,6 +8,7 @@
 import contextlib
 import glob as pyglob
 import itertools
+import math
 import os
 import re
 import sys
@@ -576,8 +577,14 @@ def calc_bound(thres):
 
         return ref*(1 + thres)
 
-    lower = calc_bound(lower_thres) or float('-inf')
-    upper = calc_bound(upper_thres) or float('inf')
+    lower = calc_bound(lower_thres)
+    if lower is None:
+        lower = -math.inf
+
+    upper = calc_bound(upper_thres)
+    if upper is None:
+        upper = math.inf
+
     try:
         evaluate(assert_bounded(val, lower, upper))
     except SanityError:


Test patch:
diff --git a/unittests/test_sanity_functions.py b/unittests/test_sanity_functions.py
index 7e6368938..0e9367027 100644
--- a/unittests/test_sanity_functions.py
+++ b/unittests/test_sanity_functions.py
@@ -473,6 +473,18 @@ def test_assert_reference():
                              r'\(l=-1\.2, u=-0\.9\)'):
         sn.evaluate(sn.assert_reference(-0.8, -1, -0.2, 0.1))
 
+    # Check that bounds are correctly calculated in case that lower bound
+    # reaches zero (see also GH issue #3430)
+    with pytest.raises(SanityError,
+                       match=r'1 is beyond reference value 0\.1 '
+                             r'\(l=0\.0, u=0\.1\)'):
+        assert sn.assert_reference(1, 0.1, -1.0, 0)
+
+    with pytest.raises(SanityError,
+                       match=r'-1 is beyond reference value -0\.1 '
+                             r'\(l=-0\.1, u=-0\.0\)'):
+        assert sn.assert_reference(-1, -0.1, 0, 1.0)
+
     # Check invalid thresholds
     with pytest.raises(SanityError,
                        match=r'invalid high threshold value: -0\.1'):