-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Add RobustMultiHelix pat. rec. #27
Conversation
Hello @brownd1978 , I think we should stick to the current organization for the fcl files. The change you proposed reflects a design that was discussed initially and we opted for a different one. As I already said, I would like to check the trigger counts when you run the new menu over the full NoPrimary1BB sample before merging this PR using a recent version of the Offline. |
Hi Gianni, this needs further discussion. Putting all the trigger
configurations in one file makes them unmaintainable and unreadable. What
possible advantage is there to this choice.
David Brown ***@***.***
Office Phone (510) 486-7261
Lawrence Berkeley National Lab
M/S 50R5008 (50-6026C) Berkeley, CA 94720
…On Fri, May 24, 2024 at 04:38 Gianantonio Pezzullo ***@***.***> wrote:
Hello @brownd1978 <https://github.com/brownd1978> , I think we should
stick to the current organization for the fcl files. The change you
proposed reflects a design that was discussed initially and we opted for a
different one.
I'm reposting the various other comments from the previous PR.
As I already said, I would like to check the trigger counts when you run
the new menu over the full NoPrimary1BB sample before merging this PR using
a recent version of the Offline.
—
Reply to this email directly, view it on GitHub
<#27 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABAH577ZL7B6UQJEHBFDHGTZD4RCBAVCNFSM6AAAAABIG6UKS6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMRZGMYTINBXGQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
I
PRs are not ideal for proposing a restructuring of the repository together with trivial changes. I see issues with your proposed re-organization that I don't think we should discuss here. |
I integrated the mpr content back into the monolithic files as requested. I don't understand the following comment:
Are you asking me to do something here? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made a few comments that should be easy to implement.
Once you made the requested changes, can you also run the triggerTest job over the NoPrimary 1BB dataset and post the trigger counts summary here?
I believe I've addressed all the comments. I ran triggerTest.fcl on NoPrimary as requested, the log file is in /nashome/b/brownd/mu2e/KFTrack/ttnoprimary.log |
Thank you @brownd1978 . I looked at the log. It looks like the rejection power is a bit limited w.r.t. the other track trigger paths, but not crazy far. |
This PR replaces PR 25 with a more modular configuration structure. I suggest making this a standard pattern in future.