Skip to content

P2 HLT timing: check on all expected json files to declare success #2662

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mmusich wants to merge 3 commits into
base: master
Choose a base branch
from
+38 −1
Open

P2 HLT timing: check on all expected json files to declare success #2662

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
84487f1
P2 HLT timing: check on all expected json files to declare success
mmusich Jan 21, 2026
fc64764
provide logs for phase HLT timing jobs
mmusich Jan 23, 2026
0e43b52
fix typ.
mmusich Jan 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 27 additions & 1 deletion pr_testing/run-pr-hlt-p2-timing.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,35 @@ popd
# Upload results
source $WORKSPACE/cms-bot/jenkins-artifacts
touch ${RESULTS_DIR}/11-hlt-p2-timing-report.res ${RESULTS_DIR}/11-hlt-p2-timing-failed.res
if [ -f $WORKSPACE/rundir/Phase2Timing_resources.json ] ; then

required_files=(
"$WORKSPACE/rundir/Phase2Timing_resources.json"
)
Copy link
Contributor

@smuzaffar smuzaffar Jan 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mmusich , do old releases also generate these files? Note that we are running these tests for CMSSW_14_1_X and above.

For IBs tests, we are uploading Phase2Timing_resources*.json files , should we change it to upload mv Phase2Timing_*.json ?

Copy link
Contributor Author

@mmusich mmusich Jan 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@smuzaffar, thank you for reviewing.

do old releases also generate these files? Note that we are running these tests for CMSSW_14_1_X and above.

This is the current situation:

  • CMSSW_16_1_X: generates all three files
  • CMSSW_16_0_X: generates all three files
  • CMSSW_15_1_X: generates Phase2Timing_resources.json and Phase2Timing_resources_NGT.json
  • CMSSW_15_0_X (and below): generates Phase2Timing_resources.json

how do you suggest to support a different number of files? I'd like to be fully explicit on the expected name of the json file: do we have means from the bash script to know the release version?

For IBs tests, we are uploading Phase2Timing_resources*.json files , should we change it to upload mv Phase2Timing_*.json ?

No. Actually thanks for pointing this out. This explains the mystery at cms-sw/cmssw#49279 (comment) !
I will rather change the name of the output json in order to match the expected pattern.
By the way can you show me which file is responsible for generating the IB tests?

Thanks!

Copy link
Contributor Author

@mmusich mmusich Jan 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looking at wha'ts done at cmssw-pr-test-config would:

CMSSW_VER=$CMSSW_VERSION 
required_files=(
    "$WORKSPACE/rundir/Phase2Timing_resources.json"
)

if [ "$CMSSW_VER" -ge 1501 ]; then
    required_files+=(
        "$WORKSPACE/rundir/Phase2Timing_resources_NGT.json"
    )
fi

if [ "$CMSSW_VER" -ge 1600 ]; then
    required_files+=(
        "$WORKSPACE/rundir/Phase2Timing_OnCPU_resources.json"
    )
fi

work?

Copy link
Contributor

@smuzaffar smuzaffar Jan 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By the way can you show me which file is responsible for generating the IB tests?

@mmusich , that code is in jenkins job itself (https://cmssdt.cern.ch/jenkins/job/ib-run-hlt-p2-timing/configure)

echo "git cms-addpkg HLTrigger/Configuration" >> $WORKSPACE/run.sh
echo "cd $WORKSPACE/$RELEASE_FORMAT/src/HLTrigger/Configuration/python/HLT_75e33/test" >> $WORKSPACE/run.sh
echo "timeout $TIMEOUT ./runHLTTiming.sh > $WORKSPACE/upload/run.log 2>&1" >> $WORKSPACE/run.sh
echo "mv Phase2Timing_resources*.json $WORKSPACE/upload" >> $WORKSPACE/run.sh

Copy link
Contributor

@smuzaffar smuzaffar Jan 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looking at wha'ts done at cmssw-pr-test-config would:

some this should work, we can always run/request PR tests for old release cycles to check if it works properly or not. Also instead of converting CMSSW_VERSION (CMSSW_16_0_X) to CMSSW_VER (1600) in p2 timing script, I would suggest to update cms-bot/pr_testing/setup-pr-test-env.sh always have CMSSW_VERSION_NUMER and use it in p2 timign script (note that setup-pr-test-env.sh is sources in all PR test jobs to may be other test can also make use of it)

Copy link
Contributor Author

@mmusich mmusich Jan 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest to update cms-bot/pr_testing/setup-pr-test-env.sh always have CMSSW_VERSION_NUMER and use it in p2 timign script (note that setup-pr-test-env.sh is sources in all PR test jobs to may be other test can also make use of it)

just to be sure I understand, the proposal is to add in cms-bot/pr_testing/setup-pr-test-env.sh this line:

CMSSW_VERSION_NUMBER=$CMSSW_VERSION`

and then structure the if / else as

if [ "$CMSSW_VERSION_NUMBER" -ge 1501 ]; then
...

?
If you confirm I'll provide a commit shortly.

Copy link
Contributor

@smuzaffar smuzaffar Jan 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

simple CMSSW_VERSION_NUMBER=$CMSSW_VERSION is not enough. You need to add something like the following or feel free to implement it with simpler logic :-) All we need is to convert major and minor version of release cycle in. to two digits and concatenate then to get CMSSW_VERSION_NUMBER

CMSSW_VERSION_NUMBER=$CMSSW_VERSION
[ "${CMSSW_VERSION_NUMBER}" != "" ] || CMSSW_VERSION_NUMBER=${RELEASE_FORMAT}
CMSSW_MAJOR=0
CMSSW_MINOR=0
if [ "${CMSSW_VERSION_NUMBER}" != "" ] ; then
  CMSSW_MAJOR=$(echo ${CMSSW_VER} | cut -d_ -f2)
  CMSSW_MINOR=$(echo ${CMSSW_VER} | cut -d_ -f3)
fi
export CMSSW_VERSION_NUMBER=$(echo x0${CMSSW_MAJOR}x0${CMSSW_MINOR} | sed -r -e 's|x[0]*([0-9][0-9])|\1|g;s|^0||')

Copy link
Contributor Author

@mmusich mmusich Jan 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tried to implement it in 84487f1.


if [ "$CMSSW_VERSION_NUMBER" -ge 1501 ]; then
required_files+=(
"$WORKSPACE/rundir/Phase2Timing_resources_NGT.json"
)
fi

if [ "$CMSSW_VERSION_NUMBER" -ge 1600 ]; then
required_files+=(
"$WORKSPACE/rundir/Phase2Timing_resources_OnCPU.json"
Copy link
Contributor Author

@mmusich mmusich Jan 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

notice that for this work, it will require to have cms-sw/cmssw#49887 integrated (and backported).

)
fi

missing=0
for f in "${required_files[@]}"; do
if [ ! -f "$f" ]; then
echo "Missing required file: $f"
missing=1
fi
done

if [ $missing -eq 0 ]; then
CHART_URL="https://cmssdt.cern.ch/circles/web/piechart.php?data_name=hlt-p2-timing&resource=time_thread&filter=${CMSSW_VERSION}&dataset=${UPLOAD_PATH}/Phase2Timing_resources"
echo "HLT_P2_TIMING;SUCCESS,HLT Phase 2 timing Test,See Chart,${CHART_URL}" >> ${RESULTS_DIR}/hlt-p2-timing.txt
echo "HLT_P2_TIMING_LOG;OK,HLT Phase 2 timing Test Log,See Logs,hlt-p2-timing.log" >> ${RESULTS_DIR}/hlt-p2-timing.txt
echo -e "**HLT P2 Timing**: [chart](${CHART_URL})" > ${RESULTS_DIR}/11-hlt-p2-timing-report.res

mv $WORKSPACE/rundir/Phase2Timing*.json $WORKSPACE/json_upload
Expand Down
11 changes: 11 additions & 0 deletions pr_testing/setup-pr-test-env.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,17 @@ SCRIPTPATH="$( cd "$(dirname "$0")" ; /bin/pwd -P )" # Absolute path to script
CMS_BOT_DIR=$(dirname ${SCRIPTPATH}) # To get CMS_BOT dir path
PR_TESTING_DIR=${CMS_BOT_DIR}/pr_testing
COMMON=${CMS_BOT_DIR}/common

CMSSW_VERSION_NUMBER=$CMSSW_VERSION
[ "${CMSSW_VERSION_NUMBER}" != "" ] || CMSSW_VERSION_NUMBER=${RELEASE_FORMAT}
CMSSW_MAJOR=0
CMSSW_MINOR=0
if [ "${CMSSW_VERSION_NUMBER}" != "" ] ; then
CMSSW_MAJOR=$(echo ${CMSSW_VERSION_NUMBER} | cut -d_ -f2)
CMSSW_MINOR=$(echo ${CMSSW_VERSION_NUMBER} | cut -d_ -f3)
fi
export CMSSW_VERSION_NUMBER=$(echo x0${CMSSW_MAJOR}x0${CMSSW_MINOR} | sed -r -e 's|x[0]*([0-9][0-9])|\1|g;s|^0||')

if [ "$WORKSPACE" = "" ] ; then export WORKSPACE=$(/bin/pwd -P) ; fi
source ${CMS_BOT_DIR}/cmsrep.sh
source ${PR_TESTING_DIR}/_helper_functions.sh
Expand Down