Merge remote-tracking branch 'origin/main' into qa-rga-fa18

.github/workflows/ci.yml

@@ -28,20 +28,16 @@ jobs:
         id: datasets
         working-directory: qadb
         run: |
-          ls -d pass*/* | jq -Rs '{"dataset": split("\n")[:-1]}' > list.json
-          echo "### List of Datasets" >> $GITHUB_STEP_SUMMARY
-          echo '```json' >> $GITHUB_STEP_SUMMARY
-          cat list.json | xargs -0 -I{} echo {} >> $GITHUB_STEP_SUMMARY
-          echo '```' >> $GITHUB_STEP_SUMMARY
-          echo datasets=$(jq -c . list.json) >> $GITHUB_OUTPUT
+          ls -d pass*/* | jq -Rs '{"dataset": split("\n")[:-1]}' | tee datasets.json
+          echo datasets=$(jq -c . datasets.json) >> $GITHUB_OUTPUT
 
   # check consistency between Groovy and C++ APIs
   test_dataset:
     needs:
       - get_datasets
     runs-on: ubuntu-latest
     strategy:
-      fail-fast: true
+      fail-fast: false
       matrix: ${{ fromJson(needs.get_datasets.outputs.datasets) }}
     steps:
       - name: checkout
@@ -58,29 +54,28 @@ jobs:
           echo "QADB=${QADB}" >> $GITHUB_ENV
           echo "JYPATH=${JYPATH}" >> $GITHUB_ENV
       - name: make sure all the table files exist # since pre-commit.ci auto-fix bot will not `git add` *new* files, we fallback to checking their existence here
-        working-directory: qadb/${{matrix.dataset}}
+        working-directory: qadb/${{ matrix.dataset }}
         run: |
           for f in qaTree.json.table miscTable.md; do
             if [ ! -f $f ]; then
               echo "missing table file '$f'; run pre-commit hook manually and commit new file(s)"
               exit 1
             fi
           done
-      - name: compile c++ tests
-        run: |
-          cd srcC/tests
-          make
+      - name: sync check
+        run: stdbuf -i0 -e0 -o0 util/syncCheck.rb ${{ matrix.dataset }}
       - name: diff the Groovy and C++ QADB dumps
         run: |
-          tests/test_diffGroovyCpp.loop.sh ${{matrix.dataset}}
+          make -C srcC/tests
+          tests/test_diffGroovyCpp.loop.sh ${{ matrix.dataset }}
       - name: concatenate artifacts
         id: artifacts
         run: |
-          mkdir -p artifacts/${{matrix.dataset}}
+          mkdir -p artifacts/${{ matrix.dataset }}
           for lang in cpp groovy ; do
-            cat tmp/${lang}*.out > artifacts/${{matrix.dataset}}/${lang}.txt ;
+            cat tmp/${lang}*.out > artifacts/${{ matrix.dataset }}/${lang}.txt ;
           done
-          echo "artifact_name=groovy_vs_cpp__$(echo ${{matrix.dataset}} | sed 's;/;_;g')" | tee -a $GITHUB_OUTPUT
+          echo "artifact_name=groovy_vs_cpp__$(echo ${{ matrix.dataset }} | sed 's;/;_;g')" | tee -a $GITHUB_OUTPUT
       - name: upload_artifacts
         uses: actions/upload-artifact@v4
         with:

src/clasqa/QADB.groovy

@@ -288,10 +288,7 @@ class QADB {
   public String getComment() { return found ? comment : "" }
   public int getEvnumMin() { return found ? evnumMin : -1 }
   public int getEvnumMax() { return found ? evnumMax : -1 }
-  public double getCharge() {
-    System.err.println "WARNING: the charge stored in the Pass 1 QADB for Run Groups A, B, K, and M is NOT quite correct, and may be off by ~1%, depending on the particular runs! This will be fixed for their Pass 2 data sets."
-    return found ? charge : -1
-  }
+  public double getCharge() { return found ? charge : -1 }
   // --- access QA info
   // check if the file has a particular defect
   // - if sector==0, checks the OR of all the sectors
@@ -468,7 +465,6 @@ class QADB {
   // -- accessor
   // call this method at the end of your event loop
   public double getAccumulatedCharge() {
-    System.err.println "WARNING: the charge stored in the Pass 1 QADB for Run Groups A, B, K, and M is NOT quite correct, and may be off by ~1%, depending on the particular runs! This will be fixed for their Pass 2 data sets."
     return chargeTotal
   }
   // reset accumulated charge, if you ever need to

src/tests/testDumpQADB.groovy

@@ -62,8 +62,11 @@ runlistFile.eachLine { runnumStr ->
 
     // check event number: report an error if evnum min>=max
     println qa.getEvnumMin() + " " + qa.getEvnumMax()
-    if(qa.getEvnumMin() >= qa.getEvnumMax())
-      err("GetEvnumMin() >= GetEvnumMax()");
+    if(qa.getEvnumMin() >= qa.getEvnumMax()) {
+      if(binnum != 0 && binnum != qa.getMaxBinnum(runnum)) { // don't bother, if not first or last bin
+        err("GetEvnumMin() >= GetEvnumMax()")
+      }
+    }
 
     // print charge (convert to pC and truncate, for easier comparison)
     // println "- charge"

srcC/include/QADB.h

@@ -126,10 +126,7 @@ namespace QA {
       inline std::string GetComment() { return found ? comment : ""; };
       inline int GetEvnumMin() { return found ? evnumMin : -1; };
       inline int GetEvnumMax() { return found ? evnumMax : -1; };
-      inline double GetCharge() {
-        std::cerr << "WARNING: the charge stored in the Pass 1 QADB for Run Groups A, B, K, and M is NOT quite correct, and may be off by ~1%, depending on the particular runs! This will be fixed for their Pass 2 data sets." << std::endl;
-        return found ? charge : -1;
-      }
+      inline double GetCharge() { return found ? charge : -1; };
       // --- access QA info
       // check if the file has a particular defect
       // - if sector==0, checks the OR of all the sectors
@@ -194,7 +191,6 @@ namespace QA {
       // returns total accumlated charge that passed your QA cuts; call this
       // method after your event loop
       inline double GetAccumulatedCharge() {
-        std::cerr << "WARNING: the charge stored in the Pass 1 QADB for Run Groups A, B, K, and M is NOT quite correct, and may be off by ~1%, depending on the particular runs! This will be fixed for their Pass 2 data sets." << std::endl;
         return chargeTotal;
       }
       // reset accumulated charge, if you ever need to

srcC/tests/testDumpQADB.cpp

@@ -68,8 +68,11 @@ int main(int argc, char ** argv) {
 
       // check event number: report an error if evnum min>=max
       cout << qa->GetEvnumMin() << " " << qa->GetEvnumMax() << endl;
-      if(qa->GetEvnumMin() >= qa->GetEvnumMax())
-        err("GetEvnumMin() >= GetEvnumMax()");
+      if(qa->GetEvnumMin() >= qa->GetEvnumMax()) {
+        if(binnum != 0 && binnum != qa->GetMaxBinnum(runnum)) { // don't bother, if not first or last bin
+          err("GetEvnumMin() >= GetEvnumMax()");
+        }
+      }
 
       // print charge (convert to pC and truncate, for easier comparison)
       // chargeInt = (int) (1000*qa->GetCharge()); // FIXME: too many warnings

util/chargeSumRuns.groovy

@@ -1,6 +1,6 @@
 // calculate total analyzed charge for a run period,
 // with specified QA cuts enabled
-// note: if syncCheck.groovy errors are present in the run range,
+// note: if syncCheck.rb errors are present in the run range,
 // the final charge value might be a bit wrong...
 
 import org.jlab.io.hipo.HipoDataSource

util/syncCheck.rb

@@ -0,0 +1,77 @@
+#!/usr/bin/env ruby
+# highlights where we have a QADB syncing problem:
+# eventnumMax of bin N is larger than eventnumMin of bin N+1
+
+require 'json'
+
+if ARGV.empty?
+  puts "USAGE: #{$0} [dataset]"
+  exit(2)
+end
+raise 'source environment variables first' if ENV['QADB'].nil?
+dataset = ARGV.first
+
+qa_tree_file = File.join ENV['QADB'], 'qadb', dataset.split('/'), 'qaTree.json'
+raise "#{qa_tree_file} does not exist" unless File.exist? qa_tree_file
+qa_tree = JSON.load_file qa_tree_file
+
+# true if any issue found
+found_issue_anywhere = false
+
+# loop over runs
+qa_tree.sort{ |a,b| a.first.to_i <=> b.first.to_i }.each do |runnum, run_tree|
+
+  binnum_prev = nil
+  found_issue_in_run = false
+
+  # loop over QA bins
+  run_tree.sort{ |a,b| a.first.to_i <=> b.first.to_i }.each do |binnum, bin_tree|
+
+    # check that min event number < max
+    evnumMin = bin_tree['evnumMin']
+    evnumMax = bin_tree['evnumMax']
+    if evnumMin > evnumMax
+      $stderr.puts "ERROR run=#{runnum} bin=#{binnum}: evnumMin > evnumMax: #{evnumMin} > #{evnumMax}"
+      found_issue_anywhere = true
+      found_issue_in_run = true
+    end
+
+    # check for overlap with previous bin
+    unless binnum_prev.nil?
+      evnumMin_prev = run_tree[binnum_prev]['evnumMin']
+      evnumMax_prev = run_tree[binnum_prev]['evnumMax']
+      if evnumMax_prev > evnumMin
+        $stderr.puts """SYNC ERROR: run=#{runnum} bin=#{binnum}: previous bin's evnumMax > this bin's evnumMin:
+  prev bin:  binnum=#{binnum_prev}\tevnumMin=#{evnumMin_prev}\tevnumMax=#{evnumMax_prev}
+  this bin:  binnum=#{binnum}\tevnumMin=#{evnumMin}\tevnumMax=#{evnumMax}"""
+        found_issue_anywhere = true
+        found_issue_in_run = true
+      end
+    end
+
+    binnum_prev = binnum
+
+  end
+
+  puts "RUN #{runnum} #{found_issue_in_run ? "has issues (see stderr)" : "is okay"}"
+end
+
+if found_issue_anywhere
+  if [ # datasets which used DST 5-files as QA bins
+      "pass1/rga_fa18_inbending",
+      "pass1/rga_fa18_outbending",
+      "pass1/rga_sp19",
+      "pass1/rgb_fa19",
+      "pass1/rgb_sp19",
+      "pass1/rgb_wi20",
+      "pass1/rgk_fa18_6.5GeV",
+      "pass1/rgk_fa18_7.5GeV",
+      "pass1/rgm_fa21",
+  ].include? dataset
+    $stderr.puts "WARNING: this dataset was done using DST 5-files as QA bins, which inherently causes SYNC ERRORS; now exitting with 0"
+    exit 0
+  else
+    $stderr.puts "ERROR: this dataset should not have any errors"
+    exit 1
+  end
+end