Suspicious branch condition in compare_eclipse.py

[bska]

Function findDeviationsThroughAllReportSteps contains the following rather curious looking condition on line 10 of output_comparator/src/compare_eclipse.py

if len(ecl_kw_list_B) != len(ecl_kw_list_B):

From context it would appear that we would need to apply the patch below. I think this is at least partially responsible for the test failures we're currently seeing on the automated Jenkins PR tests for opm-autodiff.

---

diff --git a/output_comparator/src/compare_eclipse.py b/output_comparator/src/compare_eclipse.py
index 00fd22b..ebaa757 100644
--- a/output_comparator/src/compare_eclipse.py
+++ b/output_comparator/src/compare_eclipse.py
@@ -7,7 +7,7 @@ def findDeviationsThroughAllReportSteps(absolute_deviations, relative_deviations
     ecl_kw_list_A = restart_file_A[keyword]
     ecl_kw_list_B = restart_file_B[keyword]

-    if len(ecl_kw_list_B) != len(ecl_kw_list_B):
+    if len(ecl_kw_list_A) != len(ecl_kw_list_B):
         print("Different number of report steps")
         exit(1)

[joakim-hove]

I agree that the test looks suspicious, and the supplied patch should probably be applied. I have not looked into the opm-autodiff failures (I guess I should ...) - but I have problems understanding how this can explain anything, as I see it the consequence of this bug is that something which should have failed with:

len(ecl_kw_list_B) != len(ecl_kw_list_B)

will stumble along and probably/hopefully - fail at a later stage. So - as I see it worst case scenario oof this bug would be a "False Green" - not a failing test?

[bska]

I have problems understanding how this can explain anything

It (the suspicious test) certainly does not explain everything. I'm sorry if I expressed myself to leave that impression. We are however occasionally seeing reports of the form

...
  File "output_comparator/src/compare_eclipse.py", line 16, in findDeviationsThroughAllReportSteps
    B_values_in_step_i = ecl_kw_list_B[report_step]
IndexError: list index out of range

which is explained by this issue (#94). See for instance the end of
https://ci.opm-project.org/job/opm-autodiff-PR-builder/7/console . I think what I'm arguing is that autodiff CI errors should be the result of genuine autodiff problems, and not something related to faulty solution comparison tools.