Skip to content

PyUtils + PROCTools: Add regex support to diff-root and add diff-root exclusion list support to RunTier0Tests.py

Alaettin Serhan Mete requested to merge amete/athena:master-diff-root-update-20200923 into master

This MR is very much a WIP and based on how sensitive this subject is (in general) I'm sure there'll be loads of discussion. This is one of the possible ways we can implement selective Performance Freeze checks in the CI w/ minimum code update. There are two aspects:

  • diff-root: The script already accepts, via --ignore-leaves, a list of branch names that it ignores while comparing the input files. The catch here is that this list has to contain the exact names of the branches (despite what the help text claims). There, I added regex support.
  • RunTier0Tests.py: Here, I added new functionality to better take advantage of the --ignore-leaves of diff-root. One can now point the script to a folder, where we keep one text file per output type (AOD, ESD, etc.) that contains a list of variables to be omitted while comparing the outputs. The script simply parses this file and passes it to diff-root.

How does all this come together? In the CI we run RunTier0Tests.py -n. We already keep the reference files under:

/eos/atlas/atlascerngroupdisk/data-art/grid-input/Tier0ChainTests/${q-test}/${branch}/${version}/my{AOD,ESD}.pool.root

now we're adding a set of files that contain the variables to be omitted in:

/eos/atlas/atlascerngroupdisk/data-art/grid-input/Tier0ChainTests/${q-test}/${branch}/${q-test}_{AOD,ESD}_diff-exclusion-list.txt

Then when the script runs, it reads the latter, parses the content and uses it. If nothing is found, it uses a default list of variables.

This way we have minimal code change that's also backward compatible. It keeps the interface to the qTestsTier0_required that we've been running in 21.0 for quite a while now intact. So, in principle, all we need to do is:

  • Make sure the exclusion files are OK (currently I have dummy ones where we ignore everything)
  • Turn this test on for the MR that go into Athena/master

I guess we can discuss this in detail tomorrow.

cc: @wlampl @emoyse @akraszna @fwinkl @christos @mhodgkin @goetz

Edited by Alaettin Serhan Mete

Merge request reports