Skip to content

Conversation

@ariostas
Copy link
Member

No description provided.

ferencek and others added 30 commits June 6, 2025 15:43
These modules seem to have been supersed by more generic ones:
- SiPixelCompareVertexSoA -> SiPixelCompareVertices
- SiPixel*CompareRecHitsSoA -> SiPixel*CompareRecHits
- SiPixel*CompareTrackSoA -> SiPixel*CompareTracks
- SiPixel*MonitorRecHitsSoA -> SiPixel*MonitorRecHitsSoAAlpaka
- SiPixel*MonitorTrackSoA -> SiPixel*MonitorTrackSoAAlpaka
- SiPixelMonitorVertexSoA -> SiPixelMonitorVertexSoAAlpaka
If cmsRun fails during the test, now reports what the cmsRun
logged.
The type was deprecated in C++23. Switch to using new with alignment specifier.
- switched from recursive types to fold expressions with lambdas
- use constexpr
- switch to using expressions
- adhere to non member variable naming conventions
- avoid adding useless variables
- being symmetric with the delete case was viewed as desireable based on PR discussion
- allocate only what is needed without padding at end of array as we are now allocating just bytes rather than a type using max_alignment size.
cmsbuild and others added 19 commits January 7, 2026 22:33
Removed use of deprecated std::aligned_storage
Phase 2 - Add new tracker DQM histograms showing OT cluster position in 2S modules & ladders
Remove obsolete CUDA-using modules from DQM/SiPixelHeterogeneous
…eCluster_v1

Miscellaneous fixes to SiStripApproximateCluster_v1
…gPFProducer

Fix a variable name in HLTScoutingPFProducer: from dxy(z)Error to dxy(z)Sig
…ithtime_16_0_0_pre4

Add time info to Run3ScoutingHBHERecHit
…duce_LogError_noise

Reduce LogError noise when using (E)EoR3 CPE conditions
Phase2-hgx364W Extend the tests for finding errors in iD->position and viceve  rsa for the V19 geometry
…edComment

ETL digitization: update comments on TDC window parameters
Phase 2 Tracker: Summary histograms for release validation
set `relval2025` autoHLT key to `Fake2`
btvNano: add abort condition for genParticle mother search
…ules

Add fillDescriptions to ToyModules.cc classes
@slava77
Copy link

slava77 commented Jan 12, 2026

not directly on topic, but still: computeRadiusFromThreeAnchorHits is used off-acc only to compute "T3 pt" (t5_t3_pt branch) and I'm not sure this use case is justified.

If I understand correctly, https://github.com/cms-sw/cmssw/blob/e069dffbeeeb4a0fc44beda6b3890b040afed7d7/RecoTracker/LSTCore/standalone/code/core/write_lst_ntuple.cc#L2569-L2577
the pt is computed from the first 3 hits (not from the anchor hits); is it intended or is it a bug?

  • If the intention was to have anchor hits, then it's more practical to use radius from the T3 SoA.
  • if this kind of pt is intentional , perhaps a more clear name can be used (like t5_t3_hits012_pt

this was apparently originally from @GNiendorf in #61 and the code there used anchor hits
https://github.com/SegmentLinking/cmssw/pull/61/changes#diff-94b5a9eab127b1781b75f4576abb46b96752badbbb5d7fe40efbe28dfe55b549R620
However the implementation changed in #136 https://github.com/SegmentLinking/cmssw/pull/136/changes#diff-94b5a9eab127b1781b75f4576abb46b96752badbbb5d7fe40efbe28dfe55b549R647-R654

@GNiendorf
Copy link
Member

GNiendorf commented Jan 12, 2026

It looks like this t5_t3_pt variable and t4_t3_pt are not used and can be removed. @ariostas @slava77

@ariostas ariostas force-pushed the ariostas/fix_standalone branch from e7ee3b1 to 74a8a34 Compare January 12, 2026 21:28
@ariostas ariostas changed the title Fix standalone issue with Acc concept by also accepting DevCpu Fix standalone issue with Acc concept by removing unnecessary code Jan 12, 2026
Copy link

@slava77 slava77 left a comment

Choose a reason for hiding this comment

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

only 74a8a34 is the actual change.
Looks OK

I'm not sure if a CI run is possible

@ariostas
Copy link
Member Author

The CI should still work. I'll submit it upstream once it's done.

/run standalone
/run checks

@github-actions
Copy link

The PR was built and ran successfully in standalone mode. Here are some of the comparison plots.

Efficiency vs pT comparison Efficiency vs eta comparison
Fake rate vs pT comparison Fake rate vs eta comparison
Duplicate rate vs pT comparison Duplicate rate vs eta comparison

The full set of validation and comparison plots can be found here.

Here is a timing comparison:

   Evt    Hits       MD       LS      T3       T5       pLS       pT5      pT3      TC       Reset    Event     Short             Rate
   avg     30.8    376.4    272.9    125.7     47.5    689.0     11.4    126.3    133.1    179.6      2.4    1995.0    1275.2+/- 303.4     611.8   explicit[s=4] (target branch)
   avg     30.9    377.4    272.1    125.9     47.6    683.8     11.4    125.4    133.0    180.5      1.7    1989.7    1275.0+/- 300.7     611.9   explicit[s=4] (this PR)

@slava77
Copy link

slava77 commented Jan 16, 2026

since it's taking so long to get this fix merged, I'd like to propose an update for the CI: to add a manually maintained list of fixup PRs to be added on top of the master for both the baseline and the topic test. This additional PR will be mentioned in the message for results.
@ariostas do you think this will work?

@ariostas
Copy link
Member Author

Yeah, I've been thinking of making some improvements to the CI and I was thinking of something along these lines. I'll work on that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.