Skip to content

bug Fix on the average charge computation. #47309

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

Closed
wants to merge 25 commits into from
Closed

Conversation

vmuralee
Copy link

PR description:

Correction for Average charge in Approximated cluster
  • The average charge defined as Int avgCharge = cluster.charge()/cluster.size() which doesn't return to nearest integer value in c++. The affect is shown in average charge difference between RAW and RAW' dataset.
  • The bug fixes as Int avgCharge - (cluster.charge()+cluster.size()/2)/cluster.size()
  • Details can found here slide 4
Compression of barycenter
  • Instead of storing barycenter to 16 bit Int , choose the float variable and compress the variable using std::round(cluster.barycenter() * bit_range / maxBarycenter)
  • bit_range is the range of each bit value, e.g. bit range for 16 bit is 2^16 - 1 = 65535.
  • maxBarycenter is the maximum value of barycenter which found that is 770.0.
  • Details can found here slide 9
Changing the configuration
  • For the RECO step, the configuration Configuration.Eras.Era_Run2024_pp_on_PbPb_approxSiStripCluster to reconstruct higher objects like jets and MET.

@cmsbuild
Copy link
Contributor

cms-bot internal usage

@cmsbuild
Copy link
Contributor

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-47309/43636

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

Copy link
Contributor

@mmusich mmusich left a comment

Choose a reason for hiding this comment

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

can you squash the commits to possibly 1 (and not 25)?
Also the PR is blocked by the code quality failure #47309 (comment).

from Configuration.Eras.Modifier_pp_on_PbPb_run3_cff import pp_on_PbPb_run3
from Configuration.ProcessModifiers.trackingNoLoopers_cff import trackingNoLoopers

Run3_pp_on_PbPb_approxSiStripClusters_2024 = cms.ModifierChain(Run3_2024.copyAndExclude([trackingNoLoopers]), approxSiStripClusters)
Copy link
Contributor

Choose a reason for hiding this comment

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

what's this file for?

Copy link
Author

Choose a reason for hiding this comment

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

To re-run the RECO step, where Track,jet and met are reconstructed using approximated cluster collection.

from Configuration.ProcessModifiers.approxSiStripClusters_cff import approxSiStripClusters
from Configuration.Eras.Modifier_pp_on_PbPb_run3_cff import pp_on_PbPb_run3

Run3_pp_on_PbPb_approxSiStripClusters_2024 = cms.ModifierChain(Run3_2024)#, approxSiStripClusters, pp_on_AA, pp_on_PbPb_run3)
Copy link
Contributor

Choose a reason for hiding this comment

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

what's this file for?

Copy link
Author

Choose a reason for hiding this comment

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

This was used to compare the approximated and default strip cluster collection. Anyway, it is droped in the new PR.

@mmusich
Copy link
Contributor

mmusich commented Feb 14, 2025

@icali FYI

@vmuralee
Copy link
Author

can you squash the commits to possibly 1 (and not 25)? Also the PR is blocked by the code quality failure #47309 (comment).

Yes, I created new PR for that #47367

@mmusich
Copy link
Contributor

mmusich commented Feb 18, 2025

Yes, I created new PR for that #47367

OK, then please close this one @vmuralee

@vmuralee vmuralee closed this Feb 18, 2025
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.

3 participants