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

Add BeamSpot. #363

Merged
merged 1 commit into from
Oct 7, 2021
Merged

Add BeamSpot. #363

merged 1 commit into from
Oct 7, 2021

Conversation

osschar
Copy link
Collaborator

@osschar osschar commented Oct 7, 2021

Please check default values for sigma and beam-width.

Read/Write of BeamSpot for standalone bin files still needs to be implemented.

@@ -15,7 +15,7 @@ namespace StdSeq {
// Hit processing
//=========================================================================

void LoadHits(Event &ev, EventOfHits &eoh)
void LoadHitsAndBeamSpot(Event &ev, EventOfHits &eoh)
Copy link
Collaborator

Choose a reason for hiding this comment

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

CMSSW is not using this part; Cmssw_LoadHits_Begin is used.
perhaps a separate method that can pass the beamspot is easier; it will be used as LoadDeads

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, cmssw will just call eoh.SetBeamSpot().
I thought it's cleaner to have a single method for standalone and rename it express intent.

Copy link
Collaborator

Choose a reason for hiding this comment

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

ok

@slava77
Copy link
Collaborator

slava77 commented Oct 7, 2021

I can imagine that the small test I did in trackreco/cmssw#54 is enough to confirm that this can be merged.

Any objections?

@osschar
Copy link
Collaborator Author

osschar commented Oct 7, 2021

Yes, looks good ... mayeb Mario can push his addition to this branch as well?

@slava77
Copy link
Collaborator

slava77 commented Oct 7, 2021

Yes, looks good ... mayeb Mario can push his addition to this branch as well?

I think that we can easily decompose to separate PRs.

@slava77 slava77 merged commit 0c61a7b into devel Oct 7, 2021
@osschar osschar deleted the beam-spot branch December 13, 2021 17:05
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.

2 participants