Running a test suite with additional options (-s / --sets) breaks when tests are BFB-compared with other tests

[phil-blain]

I just ran into this issue again and realized I already knew about that but had not opened an issue to document it.

In the context of #560 I ran the decomp_suite in a debug configuration, i.e. I added -s debug to the cice.setup command line.

However, it seems our testing framework is unprepared for that use case if any test in the test suite have an entry under the BFB-compare column in the <test_suite>.ts definition file. For example, for the decomp suite:

CICE/configuration/scripts/tests/decomp_suite.ts

Lines 1 to 6 in 0ac4cd9

	# Test Grid PEs Sets BFB-compare
	restart gx3 4x2x25x29x4 dslenderX2
	decomp gx3 4x2x25x29x5
	sleep 30
	restart gx3 1x1x50x58x4 droundrobin,thread restart_gx3_4x2x25x29x4_dslenderX2
	restart gx3 4x1x25x116x1 dslenderX1,thread restart_gx3_4x2x25x29x4_dslenderX2

The name of the test in the BFB-compare column is written to the ICE_BFBCOMP variable in cice.settings verbatim, i.e. it does not take into account additional options that were added on the command line. So the BFB compare part of the test fails with "missing-data" because they are not looking at the right place, i.e. the test on line 5 will look for restart_gx3_4x2x25x29x4_dslenderX2 but should look for restart_gx3_4x2x25x29x4_debug_dslenderX2.

So cice.setup should ideally add additional options from the command line before writing ICE_BFBCOMP in cice.settings for each test.

[phil-blain]

Also (that might be harder, but still I'll mention it): when adding -s debug, obviously the runs take longer (because of -O0) and so some tests go beyond their wallclock. So it would be nice to have an option to increase wallclock by some factor, something like -s double_wallclock or similar, that multiply the wallclock by 2 for each test, so that the jobs do not run out of time.

[apcraig]

It's going to be hard to support this feature. I think we'll just have to live with the fact that the bfb-compare feature will not be working. The easy alternative is to create a custom test suite file with the bfb baseline casename modified as needed.

With regard to time outs, we can already support this. Try -s debug,short or -s debug,medium. That will turn on debug and extend the wall time.

[phil-blain]

Yeah I understand it's kind of an edge case, I just wanted to open an issue to document it.

Reagrding short/medium: yes, I can use that, but I would have prefered a factor, so as not to "lie" to the scheduler (and so being scheduled later). So tests that are defined as 15 minutes become 30 minutes, those that are defined as 30 become 60, etc.

But I can live with that :)

[phil-blain]

This bit me again yesterday and I came up with this patch to cice.setup. It uses the same approach as is used to sort the sets a few lines above:

diff --git i/cice.setup w/cice.setup
index 8dc4600..cc5e6ff 100755
--- i/cice.setup
+++ w/cice.setup
@@ -644,11 +644,6 @@ EOF
       set bfbcomp = "$bfbcomp_tmp"
     endif
 
-    set fbfbcomp = ${spval}
-    if ($bfbcomp != ${spval}) then
-      set fbfbcomp = ${machcomp}_${bfbcomp}
-    endif
-
     #------------------------------------------------------------
     # Parse pesx with strict checking, limit pes for machine
 
@@ -768,6 +763,26 @@ EOF
       set testname_base = "${machcomp}_${test}_${grid}_${pesx}${soptions}.${testid}"
       set testname = "${tsdir}/${testname_base}"
       set case = ${testname}
+
+      # Get sets in bfbcomp, add sets from cice.setup argument (-s / --sets) and sort them
+      set bfbcomp_regex="\(.*_[0-9x]*\)_\(.*\)"
+      set bfbcomp_test_grid_pes=`echo ${bfbcomp} | sed "s/${bfbcomp_regex}/\1/"`
+      set bfbcomp_sets=`echo ${bfbcomp} | sed "s/${bfbcomp_regex}/\2/" | sed 's/_/,/g' `
+      set bfbcomp_sets="${bfbcomp_sets},${sets_base}"
+      set bfbcomp_soptions = ""
+      # Create sorted array and remove duplicates and "none"
+      set bfbcomp_setsarray = `echo ${bfbcomp_sets} | sed 's/,/ /g' | fmt -1 | sort -u`
+      if ("${bfbcomp_setsarray}" != "") then
+        foreach field (${bfbcomp_setsarray})
+          if (${field} != "none") then
+            set bfbcomp_soptions = ${bfbcomp_soptions}"_"${field}
+          endif
+        end
+      endif
+      set fbfbcomp = ${spval}
+      if ($bfbcomp != ${spval}) then
+        set fbfbcomp = ${machcomp}_${bfbcomp_test_grid_pes}${bfbcomp_soptions}
+      endif
     endif
 
     if (-d ${case}) then

it seems to do the trick: additional sets from the command line are correctly written to the test name appearing in ICE_BFBCOMP in cice.settings.

[apcraig]

Thanks @phil-blain, this seems like a good solution. I can include it in my next PR unless you create a PR first.

[apcraig]

Just a quick note that this seems to work well. We do need to keep the original implementation around for "cases" but the fix works well for suites.