TICL: Consolidate v5 as Default Configuration and Cleanup Legacy Code#49932
TICL: Consolidate v5 as Default Configuration and Cleanup Legacy Code#49932felicepantaleo wants to merge 29 commits intocms-sw:masterfrom
Conversation
|
cms-bot internal usage |
|
@cmsbuild please test |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-49932/47681
|
|
A new Pull Request was created by @felicepantaleo for master. It involves the following packages:
@AdrianoDee, @DickyChant, @Martin-Grunewald, @Moanwar, @antoniovagnerini, @civanch, @ctarricone, @davidlange6, @fabiocos, @ftenchini, @gabrielmscampos, @jfernan2, @kpedro88, @mandrenguyen, @mdhildreth, @miquork, @mmusich, @nothingface0, @rseidita, @srimanob can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
-1 Failed Tests: UnitTests Failed Unit TestsI found 1 errors in the following unit tests: ---> test testProduceNanoHLT had ERRORS Comparison SummarySummary:
Max Memory Comparisons exceeding threshold@cms-sw/core-l2 , I found 9 workflow step(s) with memory usage exceeding the error threshold: Expand to see workflows ...
|
|
@cmsbuild please test |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-49932/47683
|
|
@cmsbuild please test |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-49932/49092
|
|
Pull request #49932 was updated. @AdrianoDee, @DickyChant, @Martin-Grunewald, @Moanwar, @antoniovagnerini, @civanch, @ctarricone, @davidlange6, @fabiocos, @ftenchini, @gabrielmscampos, @hjkwon260, @jfernan2, @kfjack, @kpedro88, @mandrenguyen, @mdhildreth, @miquork, @mmusich, @nothingface0, @rseidita, @srimanob, @sroychow, @valsdav, @y19y19 can you please check and sign again. |
|
-1 Failed Tests: RelVals RelVals-INPUT AddOn The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic: You can see more details here: HLT P2 Timing: chart Failed RelVals
Expand to see more relval errors ...
Failed RelVals-INPUT
Failed AddOn Tests |
|
@cmsbuild please test |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-49932/49094
|
|
Pull request #49932 was updated. @AdrianoDee, @DickyChant, @Martin-Grunewald, @Moanwar, @antoniovagnerini, @civanch, @ctarricone, @davidlange6, @fabiocos, @ftenchini, @gabrielmscampos, @hjkwon260, @jfernan2, @kfjack, @kpedro88, @mandrenguyen, @mdhildreth, @miquork, @mmusich, @nothingface0, @rseidita, @srimanob, @sroychow, @valsdav, @y19y19 can you please check and sign again. |
|
-1 Failed Tests: RelVals RelVals-INPUT AddOn The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:
You can see more details here: HLT P2 Timing: chart Failed RelVals
Expand to see more relval errors ...
Failed RelVals-INPUT
Failed AddOn Tests |
fwyzard
left a comment
There was a problem hiding this comment.
A suggestion to clean up some of the HLT configuration.
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-49932/49107
|
|
Pull request #49932 was updated. @AdrianoDee, @DickyChant, @Martin-Grunewald, @Moanwar, @antoniovagnerini, @civanch, @cmsbuild, @ctarricone, @davidlange6, @fabiocos, @ftenchini, @gabrielmscampos, @hjkwon260, @jfernan2, @kfjack, @kpedro88, @mandrenguyen, @mdhildreth, @miquork, @mmusich, @nothingface0, @rseidita, @srimanob, @sroychow, @valsdav, @y19y19 can you please check and sign again. |
|
@cmsbuild please test |
|
@cmsbuild please test |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-49932/49110
|
|
Pull request #49932 was updated. @AdrianoDee, @DickyChant, @Martin-Grunewald, @Moanwar, @antoniovagnerini, @civanch, @ctarricone, @davidlange6, @fabiocos, @ftenchini, @gabrielmscampos, @hjkwon260, @jfernan2, @kfjack, @kpedro88, @mandrenguyen, @mdhildreth, @miquork, @mmusich, @nothingface0, @rseidita, @srimanob, @sroychow, @valsdav, @y19y19 can you please check and sign again. |
|
-1 Failed Tests: RelVals-INPUT The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:
You can see more details here: HLT P2 Timing: chart Failed RelVals-INPUT
Comparison SummarySummary:
Max Memory Comparisons exceeding threshold@cms-sw/core-l2 , I found 19 workflow step(s) with memory usage exceeding the error threshold: Expand to see workflows ...
|
|
@cmsbuild ignore tests-rejected with external-failure |
|
@cmsbuild please test |
|
+1 Size: This PR adds an extra 16KB to repository HLT P2 Timing: chart Comparison SummarySummary:
Max Memory Comparisons exceeding threshold@cms-sw/core-l2 , I found 19 workflow step(s) with memory usage exceeding the error threshold: Expand to see workflows ...
|
|
Is the ~100-200 MB increase in peak allocated memory along expectations? |
Memory profile of the v5 PR in ttbar + 200PUThe memory profile was studied in Step2Using The v5 PR reduces these memory peaks very substantially. The largest improvement is observed for With respect to the original v4 baseline, the PR also brings Overall, in step2 the PR removes the multi-GiB TICL memory regression introduced in v5 and reduces the dominant TICL allocations by approximately two orders of magnitude relative to the v5 release. Step3The same pattern is visible in step3. With The v5 PR strongly reduces these peaks. Relative to the v5 release, Compared to v4, the PR again brings the main CLUE3D TICL peak significantly below the v4 value: There are some non-TICL memory increases in the step3 PR. For example, The v5 PR provides a very large reduction of the dominant TICL memory peaks in both step2 and step3. The most important improvements are:
The PR therefore fixes the main TICL memory regression seen in the v5 release. In the dominant TICL modules, the PR memory usage is not only far below the v5 release, but in several cases also below the original v4 baseline. The remaining increases are comparatively smaller and mostly outside the dominant TICL memory peaks. step2: v4 as baseline
step 2: v5 (release) as baseline
step3 v4 as baseline
step3 v5 as baseline
|
at job level I have no idea, but we are doing more physics and more reconstruction, so I guess.. |
|
@mandrenguyen it would be nice to have this in |
This PR establishes TICL v5 as the default reconstruction configuration, removing the need for explicit process modifiers and cleaning up significant amounts of legacy code associated with previous versions.
Key Changes:
Core Configuration:
Updated
RecoHGCal/TICL/python/iterativeTICL_cff.pyto use the TICL v5 chain (CLUE3DHigh->TracksterLinks->TICLCandidate) by default.Removed fallback logic for v4.
Switched
CLUE3DHighStepto use PFN inference by default.Introduced
ticl_devprocess modifier for future development.C++ Plugins:
PFTICLProducer: Simplified logic by removing the
isTICLv5_switch. The default behavior now assumes TICL v5 timing fromTICLCandidate.PatternRecognition: Enabled
computeLocalTimeby default inPatternRecognitionbyCLUE3DandPatternRecognitionbyCA. EnabledusePCACleaningby default inPatternRecognitionbyCLUE3D.Validation & DQM:
Updated
HGCalValidatorto validateticlCandidateandticlTracksterLinkscollections by default.Updated
makeHGCalValidationPlots.pyto plot v5 collections.Updated
RecoHGCal_EventContent_cff.pyto consolidate keep statements for v5 collections.Fixed handling of empty collections in validator plugins.
Cleanup:
Removed obsolete process modifiers:
ticl_v4,ticl_v5,clue3D,fastJetTICL,enableCPfromPU.Removed deprecated Python configurations:
ticl_iterations.py,customiseForTICLv5_cff.py,customiseTICLFromReco.py.Removed legacy
TracksterInferenceByCNNv4implementation.Removed deprecated
harvestHGCalValidationPlots.pyscript.HLT & Workflows:
Updated HLT 75e33 and Scouting menus to use TICL v5 components.
Removed obsolete HLT modules (
hltParticleFlowSuperClusterHGCalFromTICLL1Seeded,hltParticleFlowSuperClusterHGCalFromTICLUnseeded).Updated PyReleaseValidation workflows to reflect the removal of the
ticl_v5modifier.Testing:
Notes:
This PR represents a major consolidation of the TICL configuration, simplifying the codebase and establishing a clean baseline for future developments.