Skip to content

added filter option to the HelixFilter + added track sequences for low p e-/e+ from the Stopping Target#36

Merged
brownd1978 merged 14 commits into
Mu2e:masterfrom
gianipez:tpr1
Oct 28, 2019
Merged

added filter option to the HelixFilter + added track sequences for low p e-/e+ from the Stopping Target#36
brownd1978 merged 14 commits into
Mu2e:masterfrom
gianipez:tpr1

Conversation

@gianipez

Copy link
Copy Markdown
Collaborator
  1. added an option to the HelixFilter that allows prescaling the result based on the phi of the point of closest approach; the function used results from Michael Whalen studies of the IPA e-.

  2. added IPA-calib track sequence and track sequences for selecting low-p e-/e+ generated in the Stopping Target

@brownd1978 brownd1978 self-assigned this Oct 22, 2019
@brownd1978 brownd1978 requested a review from bechenard October 22, 2019 23:49
@kutschke

Copy link
Copy Markdown
Contributor

Does he pre-scaling involve randomness?

Comment thread TrkFilters/src/HelixFilter_module.cc Outdated
TrkFitFlag _goodh; // helix fit flag
std::string _trigPath;
bool _prescaleUsingD0Phi;
std::vector<float> _prescalerPar;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A small struct would be better here - give the 3 parameters meaning names. You can write fcl helper functions that allow you to read a table into a struct with a single function call in the calling code. Let me know if you want to see an example.

@gianipez

gianipez commented Oct 23, 2019 via email

Copy link
Copy Markdown
Collaborator Author

@kutschke

kutschke commented Oct 23, 2019 via email

Copy link
Copy Markdown
Contributor

@gianipez

gianipez commented Oct 23, 2019 via email

Copy link
Copy Markdown
Collaborator Author

@@ -95,6 +101,13 @@ namespace mu2e
return true;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You could make the parameter const
int HelixFilter::evalIPAPresc(const float &phi0){

Comment thread TrkFilters/src/HelixFilter_module.cc Outdated
_goodh (pset.get<vector<string> >("helixFitFlag",vector<string>{"HelixOK"})),
_trigPath (pset.get<std::string>("triggerPath")),
_prescaleUsingD0Phi(pset.get<bool> ("prescaleUsingD0Phi",false)),
_prescalerPar (pset.get< vector<float> >("prescalerPar", vector<float>{6.2, 0.9675, 0.1396})),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please include a description in comment of what these parameters are. This would be 'automatic' if the code were upgraded to validated fcl.

@@ -1,10 +0,0 @@
MinNStrawHits 15

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is it normal that so many files get deleted in trigger commits? It makes me wonder if we're mis-using the repo.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Hi Dave,

I deleted the old .config files that are no longer used. Only the file with the list of the trigger sequences is a .config, all the others are fcl files

@bechenard bechenard left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good to me

@kutschke

kutschke commented Oct 24, 2019 via email

Copy link
Copy Markdown
Contributor

@gianipez

gianipez commented Oct 24, 2019 via email

Copy link
Copy Markdown
Collaborator Author

@gianipez

Copy link
Copy Markdown
Collaborator Author

Hi,

I implemented all the fixes needed.

Thanks for the patience,

Giani

@brownd1978 brownd1978 merged commit 1ac6768 into Mu2e:master Oct 28, 2019
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