Skip to content
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.

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

Porting the Pixel Barycentre tool in the All-in-One framework #45975

Merged
merged 8 commits into from
Oct 18, 2024

Conversation

amecca
Copy link
Contributor

@amecca amecca commented Sep 11, 2024

PR description:

This PR includes the changes necessary to run the PixelBarycentre tool from the Tracker All-in-One framework for the validation of new alignments. This allows monitoring the movements of the barycentre of BPIX (or any other structure) as part of the routine validation workflow. The outline of the proposed changes was presented at the Tracker Alignment meetings.

PR validation:

Several results were derived both with the standalone PixelBarycentre and with the new workflow, including BPIX barycentre for 2024 pp MC, and no discrepancies were observed.
Three unit tests were added to ensure that the tool is able to run with different configurations.

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 11, 2024

cms-bot internal usage

@cmsbuild
Copy link
Contributor

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-45975/41734

Code check has found code style and quality issues which could be resolved by applying following patch(s)

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @amecca for master.

It involves the following packages:

  • Alignment/OfflineValidation (alca)

@cmsbuild, @consuegs, @perrotta, @saumyaphor4252 can you please review it and eventually sign? Thanks.
@adewit, @mmusich, @rsreds, @tlampen, @tocheng, @yuanchao this is something you requested to watch as well.
@antoniovilela, @mandrenguyen, @rappoccio, @sextonkennedy you are the release manager for this.

cms-bot commands are listed here

@perrotta
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

-1

Failed Tests: Build ClangBuild
Size: This PR adds an extra 12KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-659605/41489/summary.html
COMMIT: 393e0cd
CMSSW: CMSSW_14_2_X_2024-09-11-2300/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/45975/41489/install.sh to create a dev area with all the needed externals and cmssw changes.

Build

I found compilation error when building:

/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02854/el8_amd64_gcc12/external/gcc/12.3.1-40d504be6370b5a30e3947a6e575ca28/bin/c++ -c -DGNU_GCC -D_GNU_SOURCE -DTBB_USE_GLIBCXX_VERSION=120301 -DTBB_SUPPRESS_DEPRECATED_MESSAGES -DTBB_PREVIEW_RESUMABLE_TASKS=1 -DTBB_PREVIEW_TASK_GROUP_EXTENSIONS=1 -DBOOST_SPIRIT_THREADSAFE -DPHOENIX_THREADSAFE -DBOOST_MATH_DISABLE_STD_FPCLASSIFY -DBOOST_UUID_RANDOM_PROVIDER_FORCE_POSIX -D__STDC_LIMIT_MACROS -D__STDC_FORMAT_MACROS -DDD4HEP_USE_GEANT4_UNITS=1 -DCMSSW_GIT_HASH='CMSSW_14_2_X_2024-09-11-2300' -DPROJECT_NAME='CMSSW' -DPROJECT_VERSION='CMSSW_14_2_X_2024-09-11-2300' -Isrc -Ipoison -I/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02854/el8_amd64_gcc12/cms/cmssw-patch/CMSSW_14_2_X_2024-09-11-2300/src -I/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02854/el8_amd64_gcc12/cms/coral/CORAL_2_3_21-f8c51f7b8431559c128f84a138675a9f/include/LCG -I/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02854/el8_amd64_gcc12/external/dd4hep/v01-29-00-72e61189ceef14e8db0f50d3b9434ade/include -I/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02854/el8_amd64_gcc12/external/alpaka/1.1.0-4d4f1220bfca9be4c4149ab758d15463/include -I/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02854/el8_amd64_gcc12/external/classlib/3.1.3-c6b100380ee001a2bc06975bb0682ffc/include -I/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02854/el8_amd64_gcc12/external/libpng/1.6.37-e9fc63ee14d46b2aa66f88196e69e632/include -I/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02854/el8_amd64_gcc12/external/pcre/8.43-e34796d17981e9b6d174328c69446455/include -isystem/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02854/el8_amd64_gcc12/external/boost/1.80.0-7ac88d918a6883d593abc133ee0eb2e9/include -I/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02854/el8_amd64_gcc12/external/bz2lib/1.0.6-d065ccd79984efc6d4660f410e4c81de/include -isystem/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02854/el8_amd64_gcc12/external/clhep/2.4.7.1-17c7283747b6cbc68a3534cb987a890c/include -I/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02854/el8_amd64_gcc12/external/cuda/12.4.1-fc5cb0e72dba64b6abbf00089f3a044c/include -I/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02854/el8_amd64_gcc12/external/curl/7.79.0-abae79fca02c8e83ab0f428f2a91d96b/include -I/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02854/el8_amd64_gcc12/external/giflib/5.2.0-07dfc72586a7288f078c7a02c8b17956/include -I/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02854/el8_amd64_gcc12/external/gsl/2.6-4e597de26ced42091883404fa44cef2e/include -I/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02854/el8_amd64_gcc12/external/hepmc/2.06.10-6a65fcd1f028c097f0f24e8fa61d1b1b/include -isystem/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02854/el8_amd64_gcc12/external/hls/2019.08-e6beae7d560007d8bb20c2cf88bfde9a/include -I/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02854/el8_amd64_gcc12/external/hls4mlEmulatorExtras/1.1.3-7abf6dadd3e09d4bf89fb5c47d56db40/include -I/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02854/el8_amd64_gcc12/external/libjpeg-turbo/2.0.2-919c9f157a4ae0be842b4ff2a75a3387/include -I/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02854/el8_amd64_gcc12/external/libuuid/2.34-27ce4c3579b5b1de2808ea9c4cd8ed29/include -I/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02854/el8_amd64_gcc12/external/protobuf/3.21.9-cceae2b21362e2ebbf12522a68c60cb1/include -isystem/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02854/el8_amd64_gcc12/lcg/root/6.30.09-5371975256c236eab45164692fccb0e2/include -I/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02854/el8_amd64_gcc12/external/sqlite/3.36.0-49157ebb9a846c99702be39683b0c263/include -isystem/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02854/el8_amd64_gcc12/external/tbb/v2021.9.0-2391c941213c757dc9a1835b31681235/include -I/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02854/el8_amd64_gcc12/external/utm/utm_0.13.0-63d7eccd27aab78c24f2de8a4081edbc/include -I/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02854/el8_amd64_gcc12/cms/vdt/0.4.3-820b4286c899d81142fb8526a68cf80f/include -I/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02854/el8_amd64_gcc12/external/xerces-c/3.1.3-c7b88eaa36d0408120f3c29826a04bf6/include -I/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02854/el8_amd64_gcc12/external/xz/5.2.5-6f3f49b07db84e10c9be594a1176c114/include -I/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02854/el8_amd64_gcc12/external/zlib/1.2.11-1a082fc322b0051b504cc023f21df178/include -I/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02854/el8_amd64_gcc12/external/eigen/3bb6a48d8c171cf20b5f8e48bfb4e424fbd4f79e-39786ff94f8aa2f543922cad720e1b32/include -I/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02854/el8_amd64_gcc12/external/eigen/3bb6a48d8c171cf20b5f8e48bfb4e424fbd4f79e-39786ff94f8aa2f543922cad720e1b32/include/eigen3 -I/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02854/el8_amd64_gcc12/external/fmt/10.2.1-deba157dd76fed17ac8b42adc60ff53f/include -I/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02854/el8_amd64_gcc12/external/hepmc3/3.2.7-c29b1572175a7bf793f46dc34d1f656a/include -I/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02854/el8_amd64_gcc12/external/md5/1.0.0-5b594b264e04ae51e893b1d69a797ec6/include -I/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02854/el8_amd64_gcc12/external/OpenBLAS/0.3.27-6574458f8642b299ac965c654e194a8b/include -isystem/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02854/el8_amd64_gcc12/external/tensorflow/2.12.0-246b418e707c33296ea454aba2cb9260/include -I/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02854/el8_amd64_gcc12/external/tinyxml2/6.2.0-68d511689c5dbd9a0d296bbb795fa052/include -O3 -pthread -pipe -Werror=main -Werror=pointer-arith -Werror=overlength-strings -Wno-vla -Werror=overflow -std=c++20 -ftree-vectorize -Werror=array-bounds -Werror=format-contains-nul -Werror=type-limits -fvisibility-inlines-hidden -fno-math-errno --param vect-max-version-for-alias-checks=50 -Xassembler --compress-debug-sections -Wno-error=array-bounds -Warray-bounds -fuse-ld=bfd -march=x86-64-v2 -felide-constructors -fmessage-length=0 -Wall -Wno-non-template-friend -Wno-long-long -Wreturn-type -Wextra -Wpessimizing-move -Wclass-memaccess -Wno-cast-function-type -Wno-unused-but-set-parameter -Wno-ignored-qualifiers -Wno-unused-parameter -Wunused -Wparentheses -Werror=return-type -Werror=missing-braces -Werror=unused-value -Werror=unused-label -Werror=address -Werror=format -Werror=sign-compare -Werror=write-strings -Werror=delete-non-virtual-dtor -Werror=strict-aliasing -Werror=narrowing -Werror=unused-but-set-variable -Werror=reorder -Werror=unused-variable -Werror=conversion-null -Werror=return-local-addr -Wnon-virtual-dtor -Werror=switch -fdiagnostics-show-option -Wno-unused-local-typedefs -Wno-attributes -Wno-psabi -DEIGEN_DONT_PARALLELIZE -DEIGEN_MAX_ALIGN_BYTES=64 -Wno-error=unused-variable -DALPAKA_DEFAULT_HOST_MEMORY_ALIGNMENT=128 -DALPAKA_DISABLE_VENDOR_RNG -DBOOST_DISABLE_ASSERTS -flto=auto -fipa-icf -flto-odr-type-merging -fno-fat-lto-objects -Wodr -fPIC -MMD -MF tmp/el8_amd64_gcc12/src/Alignment/OfflineValidation/plugins/AlignmentOfflinevalidationPlugins/SagittaBiasNtuplizer.cc.d src/Alignment/OfflineValidation/plugins/SagittaBiasNtuplizer.cc -o tmp/el8_amd64_gcc12/src/Alignment/OfflineValidation/plugins/AlignmentOfflinevalidationPlugins/SagittaBiasNtuplizer.cc.o
src/Alignment/OfflineValidation/plugins/PixelBaryCentreAnalyzer.cc: In member function 'virtual void PixelBaryCentreAnalyzer::analyze(const edm::Event&, const edm::EventSetup&)':
src/Alignment/OfflineValidation/plugins/PixelBaryCentreAnalyzer.cc:250:28: warning: unused variable 'tkTopo' [-Wunused-variable]
  250 |     const TrackerTopology& tkTopo = iSetup.getData(trackerTopologyToken_);
      |                            ^~~~~~
src/Alignment/OfflineValidation/plugins/PixelBaryCentreAnalyzer.cc:311:21: error: 'tkTopo' was not declared in this scope
  311 |         int layer = tkTopo.pxbLayer(detId);
      |                     ^~~~~~
src/Alignment/OfflineValidation/plugins/PixelBaryCentreAnalyzer.cc:324:20: error: 'tkTopo' was not declared in this scope
  324 |         int disk = tkTopo.pxfDisk(detId);
      |                    ^~~~~~


Clang Build

I found compilation error while trying to compile with clang. Command used:

USER_CUDA_FLAGS='--expt-relaxed-constexpr' USER_CXXFLAGS='-Wno-register -fsyntax-only' scram build -k -j 32 COMPILER='llvm compile'

>> Local Products Rules ..... started
>> Local Products Rules ..... done
>> Creating project symlinks
>> Entering Package Alignment/OfflineValidation
>> Compile sequence completed for CMSSW CMSSW_14_2_X_2024-09-11-2300
gmake: *** [There are compilation/build errors. Please see the detail log above.] Error 1
+ eval scram build outputlog '&&' '(python3' /data/cmsbld/jenkins/workspace/ib-run-pr-tests/cms-bot/buildLogAnalyzer.py --logDir /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_14_2_X_2024-09-11-2300/tmp/el8_amd64_gcc12/cache/log/src '||' 'true)'
++ scram build outputlog
>> Entering Package Alignment/OfflineValidation
>> Compiling  src/Alignment/OfflineValidation/bin/DMRmerge.cc
>> Compiling  src/Alignment/OfflineValidation/bin/Options.cc


phase_ = 1;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

there is something funny with parenthesis going on here. I think this is superfluous now and this clause should actually be closed at L545 of the new layout.

} // check for new IOV for TKAlign

This also explains why the formatting of the following piece:

if (prepareBS) {
// loop over bsLabels_
for (const auto& label : bsLabels_) {
// init bstree content
PixelBaryCentreAnalyzer::initBS();
// Get BeamSpot from EventSetup
const BeamSpotObjects* mybeamspot = &iSetup.getData(bsTokens_[label]);
BS_ = GlobalPoint(mybeamspot->x(), mybeamspot->y(), mybeamspot->z());
vBS_ = SimplePoint(BS_);
bsTrees_[label]->Fill();
} // bsLabels_
} // check for new IOV for BS
}

is odd.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, I decided to fix it in the first commit and rebase

@cmsbuild
Copy link
Contributor

@TomasKello
Copy link
Contributor

Hi @amecca gentle reminder. Better to close this asap.

@amecca
Copy link
Contributor Author

amecca commented Oct 16, 2024

Hi @mmusich and @TomasKello, sorry for waiting so long. The Jenkins logs are not available anymore, and I had problems running the unit tests offline. I remember that the failing test was Alignment/OfflineValidation/Miscellanea, and I fixed it. However, I am not able to check offline if all of the other tests pass.

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Pull request #45975 was updated. @atpathak, @cmsbuild, @consuegs, @perrotta can you please check and sign again.

@mmusich
Copy link
Contributor

mmusich commented Oct 16, 2024

please test

@cmsbuild
Copy link
Contributor

+1

Size: This PR adds an extra 20KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-659605/42246/summary.html
COMMIT: ae31a0f
CMSSW: CMSSW_14_2_X_2024-10-15-2300/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/45975/42246/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

@perrotta
Copy link
Contributor

+alca

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @antoniovilela, @mandrenguyen, @sextonkennedy, @rappoccio (and backports should be raised in the release meeting by the corresponding L2)

@mandrenguyen
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit a503731 into cms-sw:master Oct 18, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants