Agnostic helix finder and correction of the charged pion lifetime #1210

NamithaChitrazee · 2024-03-06T17:39:10Z

This PR has the following main updates:

Correcting the charged pion lifetime by @pavel1murat
A helix finder by @matthewstortini
A filter stage in TZClusterFinder by Alessandro Ricci

Will use it to create the filtered cosmics low dataset.

…luster small update for extracted position in EventDisplay

… the number 2 with a constexpr, replace some of the vectors with std::arrays

Changes to KinKal and CRV (PR1073) incorporated while still keeping the build e20-p047

Merge p055

brownd1978 · 2024-05-04T23:25:35Z

@FNALbuild run build test

FNALbuild · 2024-05-04T23:26:01Z

⌛ The following tests have been triggered for d99670e: build (Build queue is empty)

FNALbuild · 2024-05-04T23:53:18Z

☀️ The build tests passed at d99670e.

Test	Result	Details
test with	✅	Command did not list any other PRs to include
merge	✅	Merged `d99670e` at `07a76f5`
build (prof)	✅	Log file. Build time: 11 min 01 sec
ceSimReco	✅	Log file.
g4test_03MT	✅	Log file.
transportOnly	✅	Log file.
POT	✅	Log file.
g4study	✅	Log file.
cosmicSimReco	✅	Log file.
cosmicOffSpill	✅	Log file.
ceSteps	✅	Log file.
ceDigi	✅	Log file.
muDauSteps	✅	Log file.
ceMix	✅	Log file.
rootOverlaps	✅	Log file.
g4surfaceCheck	✅	Log file.
FIXME, TODO	🔶	TODO (2) FIXME (13) in 28 files
clang-tidy	🔶	0 errors 2145 warnings
whitespace check	✅	no whitespace errors found

N.B. These results were obtained from a build of this Pull Request at d99670e after being merged into the base branch at 07a76f5.

For more information, please check the job page here.
Build artifacts are deleted after 5 days. If this is not desired, select Keep this build forever on the job page.

brownd1978 · 2024-05-05T00:52:14Z

Once Production PR 316 is merged I will merge this PR. Note that a number of issues related to changes requested but not implemented in this PR have been added to Offline. It is expected that those will be dealt with promptly.

kutschke

I agree that it's right to merge this and create issues to deal with any outstanding work.

FNALbuild · 2024-05-05T04:15:55Z

📝 The HEAD of main has changed to f6fe4b1. Tests are now out of date.

kutschke · 2024-05-05T13:20:24Z

@FNALbuild run build test

FNALbuild · 2024-05-05T13:20:50Z

⌛ The following tests have been triggered for d99670e: build (Build queue is empty)

gianipez · 2024-05-05T13:33:54Z

Thanks a lot!

FNALbuild · 2024-05-05T13:48:07Z

☀️ The build tests passed at d99670e.

Test	Result	Details
test with	✅	Command did not list any other PRs to include
merge	✅	Merged `d99670e` at `f6fe4b1`
build (prof)	✅	Log file. Build time: 11 min 07 sec
ceSimReco	✅	Log file.
g4test_03MT	✅	Log file.
transportOnly	✅	Log file.
POT	✅	Log file.
g4study	✅	Log file.
cosmicSimReco	✅	Log file.
cosmicOffSpill	✅	Log file.
ceSteps	✅	Log file.
ceDigi	✅	Log file.
muDauSteps	✅	Log file.
ceMix	✅	Log file.
rootOverlaps	✅	Log file.
g4surfaceCheck	✅	Log file.
FIXME, TODO	🔶	TODO (2) FIXME (13) in 28 files
clang-tidy	🔶	0 errors 2145 warnings
whitespace check	✅	no whitespace errors found

N.B. These results were obtained from a build of this Pull Request at d99670e after being merged into the base branch at f6fe4b1.

For more information, please check the job page here.
Build artifacts are deleted after 5 days. If this is not desired, select Keep this build forever on the job page.

brownd1978 · 2024-05-06T03:22:02Z

I cannot merge this PR until gaponenko removes his request for changes. If he doesn't wish to approve he can simply downgrade his review to a comment.

NamithaChitrazee · 2024-05-06T15:17:55Z

Hi @brownd1978 , @kutschke

Thank you very much.

gaponenko · 2024-05-06T17:22:26Z

I cannot merge this PR until gaponenko removes his request for changes. If he doesn't wish to approve he can simply downgrade his review to a comment.

Hi Dave, I thought I already did that. My previous post on this PR #1210 (review) is a comment. I don't see what else I can do, as the options are "request changes", "comment" (which I did), or "approve". I already explained why I did not want to approve.
How can I help to remove the "block" without doing the approval?

brownd1978 · 2024-05-06T19:32:59Z

Hi Andrei,

On Mon, May 6, 2024 at 10:22 AM Andrei Gaponenko ***@***.***> wrote: I cannot merge this PR until gaponenko <https://github.com/gaponenko> removes his request for changes. If he doesn't wish to approve he can simply downgrade his review to a comment. Hi Dave, I thought I already did that. My previous post on this PR #1210 (review) <#1210 (review)> is a comment. I don't see what else I can do, as the options are "request changes", "comment" (which I did), or "approve". I already explained why I did not want to approve. How can I help to remove the "block" without doing the approval?

To supersede your previous review go to the PR select 'files changed' and the select the green 'Review changes' button. From there you can change your review to a comment, then 'submit review' thanks, Dave

…

— Reply to this email directly, view it on GitHub <#1210 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABAH572226LBPXTJFA4DXMDZA636RAVCNFSM6AAAAABEJP7CI6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAOJWGU2DCMJSG4> . You are receiving this because you were mentioned.Message ID: ***@***.***>

-- David Brown ***@***.*** Office Phone (510) 486-7261 Lawrence Berkeley National Lab M/S 50R5008 (50-6026C) Berkeley, CA 94720

gaponenko

Changing to a comment

brownd1978 · 2024-05-06T20:38:16Z

Hi Andrei, it turns out I'm wrong: simply commenting in the review response is not enough to remove a 'changes requested' review. You must either approve the PR, or dismiss your 'change request'. sorry for the confusion. Dave

…

On Mon, May 6, 2024 at 1:17 PM Andrei Gaponenko ***@***.***> wrote: ***@***.**** commented on this pull request. Changing to a comment — Reply to this email directly, view it on GitHub <#1210 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABAH572FULJV3GHB5ANWUEDZA7QNTAVCNFSM6AAAAABEJP7CI6VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDANBRGYYDEMZSGQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

-- David Brown ***@***.*** Office Phone (510) 486-7261 Lawrence Berkeley National Lab M/S 50R5008 (50-6026C) Berkeley, CA 94720

Dave and I understand Andrei's objections to approving this PR and we are keeping them on the record. There is no practical way to satisfy Andrei's requests at this time so I will dismiss the review and proceed with the merge.

kutschke · 2024-05-06T22:07:16Z

Dismissing Andrei's review indeed preserved the full history of his comments and requests for changes. You may need to hit the "Load more" button to see them.

To the authors: @NamithaChitrazee @pavel1murat @matthewstortini @Hiigaran The issues #1250 and #1251 were created for you to follow up on issues raised during the review but not yet resolved. There is also work requested on the event generator that was removed from the review.

NamithaChitrazee and others added 30 commits August 20, 2023 14:03

filter based on the PID of the helix

340b866

Will use it to create the filtered cosmics low dataset.

Update prolog.fcl

23c68d3

added average hit times for both readout sides of a CRV coincidence c…

4239a11

…luster small update for extracted position in EventDisplay

fcl file for extracted position in EventDisplay

7c37ef7

changes related to CRV coincidence finder: remove std::pairs, replace…

e18faa2

… the number 2 with a constexpr, replace some of the vectors with std::arrays

bug fixes

787c321

reconstruct CRV hit position based on readout times at both counter ends

5561b6b

comment about fiberSignalSpeed

0699253

Merge branch 'pbar2m' into pbar2m3

db36f13

Merge pull request #11 from NamithaChitrazee/pbar2m3

2c379aa

Changes to KinKal and CRV (PR1073) incorporated while still keeping the build e20-p047

Fix config bug

393ec80

putting in framework to filter out bad TCs for Ethan to work on

945813e

fix charges pion lifetime

69dfe24

add more columns to 1-like-per object printout

ae54f21

extend generator_tool to return its process code

0be99c5

fit charged pion lifetime

235da9d

merging offline p051, fix tabs and whitespaces

b953053

fix indentation

bdb7ecc

starting point for Hussein

071f641

bug fix

e2f976a

fix compilation in prof mode

dc4a23a

Merge remote-tracking branch 'origin/main' into merge_051

d98351f

Merge branch 'main' into merge_051

a673c96

small changes

5ee711e

Merge branch 'pbar2m' into merge_051

0ebcd55

Merge pull request #13 from NamithaChitrazee/merge_051

bec3987

Merge p055

added the option to run filters without filtering

39a7a61

update comments

13b447c

generate pi+ --> enu

a57ad67

generate pi+ --> enu

697c370

FNALbuild added build running and removed build pending labels May 4, 2024

FNALbuild added build finished and removed build running labels May 4, 2024

kutschke approved these changes May 5, 2024

View reviewed changes

FNALbuild added build pending and removed build finished labels May 5, 2024

FNALbuild added build running and removed build pending labels May 5, 2024

FNALbuild added build finished and removed build running labels May 5, 2024

Mu2e deleted a comment from kutschke May 6, 2024

gaponenko reviewed May 6, 2024

View reviewed changes

kutschke merged commit 7eeafee into Mu2e:main May 6, 2024
14 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Agnostic helix finder and correction of the charged pion lifetime #1210

Agnostic helix finder and correction of the charged pion lifetime #1210

NamithaChitrazee commented Mar 6, 2024 •

edited

Loading

brownd1978 commented May 4, 2024

FNALbuild commented May 4, 2024

FNALbuild commented May 4, 2024

brownd1978 commented May 5, 2024 •

edited

Loading

kutschke left a comment

FNALbuild commented May 5, 2024

kutschke commented May 5, 2024

FNALbuild commented May 5, 2024

gianipez commented May 5, 2024

FNALbuild commented May 5, 2024

brownd1978 commented May 6, 2024

NamithaChitrazee commented May 6, 2024

gaponenko commented May 6, 2024

brownd1978 commented May 6, 2024 via email

gaponenko left a comment

brownd1978 commented May 6, 2024 via email

kutschke commented May 6, 2024

Agnostic helix finder and correction of the charged pion lifetime #1210

Agnostic helix finder and correction of the charged pion lifetime #1210

Conversation

NamithaChitrazee commented Mar 6, 2024 • edited Loading

brownd1978 commented May 4, 2024

FNALbuild commented May 4, 2024

FNALbuild commented May 4, 2024

brownd1978 commented May 5, 2024 • edited Loading

kutschke left a comment

Choose a reason for hiding this comment

FNALbuild commented May 5, 2024

kutschke commented May 5, 2024

FNALbuild commented May 5, 2024

gianipez commented May 5, 2024

FNALbuild commented May 5, 2024

brownd1978 commented May 6, 2024

NamithaChitrazee commented May 6, 2024

gaponenko commented May 6, 2024

brownd1978 commented May 6, 2024 via email

gaponenko left a comment

Choose a reason for hiding this comment

brownd1978 commented May 6, 2024 via email

kutschke commented May 6, 2024

NamithaChitrazee commented Mar 6, 2024 •

edited

Loading

brownd1978 commented May 5, 2024 •

edited

Loading