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

Fxt offset vtx #720

Merged
merged 6 commits into from
Dec 10, 2024
Merged

Fxt offset vtx #720

merged 6 commits into from
Dec 10, 2024

Conversation

genevb
Copy link
Contributor

@genevb genevb commented Dec 10, 2024

This PR adjusts the selection of candidate tracks for use in Minuit vertex-finding to be offset by -2 cm from the z-axis when processing with the FXT chain option. The z-axis is otherwise the default for the Minuit vertex-finding RImpact cut.

@fisyak
Copy link
Member

fisyak commented Dec 10, 2024

Thie proposed  modification does not account a shift in Z position o fintecept with beam line.

I proposed for FXT doL

  •  static Double_t xBeam = 0;
    
  •  static Double_t yBeam = -2.
    
  •  THelixTrack thelix = gDCA->thelix();
    
  •  Double_t step = thelix.Path(xBeam, yBeam);
    
  •  Double_t xyz[3];
    
  •  thelix.Eval(step, xyz);
    
  •  Double_t dcaXY = TMath::Sqrt((xyz[0] - xBeam)*(xyz[0] - xBeam) + (xyz[1] - yBeam)*(xyz[1] - yBeam));
    
  •  if (dcaXY  >  mRImpactMax) continue;
    
  •  Double_t z_lin = xyz[2];
    

Copy link
Member

@plexoos plexoos left a comment

Choose a reason for hiding this comment

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

Looks good!

Not sure whether the generated files should be included in the diff, though.

@genevb
Copy link
Contributor Author

genevb commented Dec 10, 2024

Not sure whether the generated files should be included in the diff, though.

Can you please remind me how to remove them, @plexoos ?

And I think @fisyak is correct that the z_lin variable needs modified. I'll update that before merging.

@genevb
Copy link
Contributor Author

genevb commented Dec 10, 2024

@plexoos , nevermind on the file exclusion...I found instructions via internet search.

The commit I just made is mathematically identical to the proposal from @fisyak, and is also mathematically identical to the prior version for the calculation of the RImpact variable (I ran multiple tests with and without the -2 cm bias to confirm this).

Thanks, @fisyak and @plexoos. I'll wait for some CI checks, and then I will squash and merge.

@genevb genevb merged commit 640288b into star-bnl:main Dec 10, 2024
148 checks passed
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.

4 participants