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

Function to set tracker bounds in KalmanVertexFitter #47454

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

kerstinlovisa
Copy link

PR description:

Including a function in KalmanVertexFitter.h to be able to change the tracker bounds in the Sequential Vertex fitter, with two additions:

  • function "setTrackerBounds" which calls the function in SequentialVertexFitter.h to update the tracker bounds.
  • removal of const for theSequentialFitter - needed in order to update the tracker bounds.

This is useful for analyses using the KalmanVertexFitter.h for displaced vertices, and we plan to use it in the first version of the EXOnanoAOD.

PR validation:

The code was tested on a 2023 MC sample. Using the function to extend the tracker bounds we get displaced vertices with outside of the default tracker bounds.

@jniedzie @kakwok @enibigir

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 26, 2025

cms-bot internal usage

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

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

It involves the following packages:

  • RecoVertex/KalmanVertexFit (reconstruction)

@cmsbuild, @jfernan2, @mandrenguyen can you please review it and eventually sign? Thanks.
@GiacomoSguazzoni, @VinInn, @VourMa, @dgulhan, @fabiocos, @martinamalberti, @missirol, @mmusich, @mtosi, @rovere 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

@slava77
Copy link
Contributor

slava77 commented Feb 26, 2025

how often will the bounds need to be changed? I'm trying to understand why making another instance is not an option.

@kerstinlovisa
Copy link
Author

how often will the bounds need to be changed? I'm trying to understand why making another instance is not an option.

The way I use this now is to create a kalman vertex fitter and set the tracker bounds in an EDProducer (which creates a collection of vertex fits):

KalmanVertexFitter vertexFitter(true);
vertexFitter.setTrackerBounds(newTrackerBoundsRadius, newTrackerBoundsHalfLenght);

If you think there's a more optimal way to change the default tracker bound values, I can try it out instead

@slava77
Copy link
Contributor

slava77 commented Feb 26, 2025

vertexFitter.setTrackerBounds(newTrackerBoundsRadius, newTrackerBoundsHalfLenght);

(let me rephrase the question to say more explicitly what I meant): how many different values of newTrackerBoundsRadius and newTrackerBoundsHalfLenght are going to be used?

If it can be just one set of values, then the way to follow the current/existing logic is to configure another fitter with these values.

@kerstinlovisa
Copy link
Author

If it can be just one set of values, then the way to follow the current/existing logic is to configure another fitter with these values.

I see, yes it can just be one set of values because we want to be able to extend the values for the muon system. I could call theSequentialFitter->setTrackerBounds(trackerBoundsRadius, trackerBoundsHalfLength); in the KalmanVertexFitter constructor instead, with extra input parameters for the tracker bounds.
From what I can see another option is to include trackerBoundsRadius and trackerBoundsHalfLength in pSet, and this would also requiere changes in SequentialVertexFitter.h.

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Pull request #47454 was updated. @cmsbuild, @jfernan2, @mandrenguyen can you please check and sign again.

@kerstinlovisa
Copy link
Author

I updated the KalmanVertexFitter with another option to use extended tracker bounds to include the muon system, please let me know if you this would be better solution

Comment on lines +113 to +114
float muonSystemBoundsRadius{740.};
float muonSystemBoundsHalfLength{960.};
Copy link
Contributor

Choose a reason for hiding this comment

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

looking back to #40479 where this started to be addressed, I'd add constexpr float kMuonSystemBoundsRadius; in the SequentialVertexFitter and perhaps set it with a SequentialVertexFitter::setMuonTrackerBounds() method instead of a fully free.

The rationale is that the issue seems to be related to SequentialVertexFitter and it's used in more than just KalmanVertexFitter and apparently the other choice of parameters is well defined.

Copy link
Author

Choose a reason for hiding this comment

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

Sorry @slava77 I don't think I understand your suggestion. Do you mean to create kMuonSystemBoundsRadius without default values, then use SequentialVertexFitter::setMuonTrackerBounds() to give values to kMuonSystemBoundsRadius? In this case I would need one more function that replaces the default trackerBoundsRadius with kMuonSystemBoundsRadius afterwards.

Or do you mean to set default values to kMuonSystemBoundsRadius, and then use SequentialVertexFitter::setMuonTrackerBounds() to replace the trackerBoundsRadius with the kMuonSystemBounds values?

We could also set up a chat if you'd want, if it would be easier to explain that way.

Copy link
Contributor

Choose a reason for hiding this comment

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

I meant to move the definition of the constants to SequentialVertexFitter and add a SequentialVertexFitter::setMuonTrackerBounds method

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