Improve segment accessors

[brownd1978]

Resolve some long-standing inconsistencies WRT t0 and provide friendlier functions for finding basic fit information

[FNALbuild]

Hi @brownd1978, You have proposed changes to files in these packages:

• TEveEventDisplay
• RecoDataProducts

which require these tests: build.

@Mu2e/write, @Mu2e/fnalbuild-users have access to CI actions on main.

:hourglass: The following tests have been triggered for bc9a12d7bec460c8638c88e33034a78592061b6f: build (Build queue is empty)

About FNALbuild. Code review on Mu2e/Offline.

[FNALbuild]

:sunny: The build tests passed at bc9a12d7bec460c8638c88e33034a78592061b6f.

Test	Result	Details
test with	:white_check_mark:	Command did not list any other PRs to include
merge	:white_check_mark:	Merged bc9a12d7bec460c8638c88e33034a78592061b6f at 2df22a8d10bd7c46d9e3c643ce0a185ac5896990
build (prof)	:white_check_mark:	Log file. Build time: 04 min 09 sec
ceSimReco	:white_check_mark:	Log file.
g4test_03MT	:white_check_mark:	Log file.
transportOnly	:white_check_mark:	Log file.
POT	:white_check_mark:	Log file.
g4study	:white_check_mark:	Log file.
cosmicSimReco	:white_check_mark:	Log file.
cosmicOffSpill	:white_check_mark:	Log file.
ceSteps	:white_check_mark:	Log file.
ceDigi	:white_check_mark:	Log file.
muDauSteps	:white_check_mark:	Log file.
ceMix	:white_check_mark:	Log file.
rootOverlaps	:white_check_mark:	Log file.
g4surfaceCheck	:white_check_mark:	Log file.
FIXME, TODO	:large_orange_diamond:	TODO (0) FIXME (5) in 5 files
clang-tidy	:large_orange_diamond:	0 errors 259 warnings
whitespace check	:white_check_mark:	no whitespace errors found

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

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]

This change is downstream of reco, it just adds options and improves accès for TrkAna and REve

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

On Mon, Jun 17, 2024 at 19:30 Rob Kutschke @.***> wrote:

@.**** commented on this pull request.

Does anyone else need to review this?

In RecoDataProducts/inc/KalSeed.hh https://github.com/Mu2e/Offline/pull/1288#discussion_r1643647112:

@@ -53,7 +53,8 @@ namespace mu2e { KinKal::TimeRange timeRange() const { return KinKal::TimeRange(_segments.front()._tmin,_segments.back()._tmax); } bool hasCaloCluster() const { return _chit.caloCluster().isNonnull(); } art::Ptr const& caloCluster() const { return _chit.caloCluster(); }

• std::vector::const_iterator nearestSeg(double time) const;
• std::vector::const_iterator nearestSegment(double time) const;
• std::vector::const_iterator t0Segment(double& t0) const; // return the segment associated with the t0 value, or the closest to it. Also return the t0 value

Have you considered returning a small struct that hods the iterator and t0? It's more obvious to a new user than a return argument. And I expect that this function will be used by new users. My suggestion is slightly more expensive than yours but I doubt that's important in this context.

— Reply to this email directly, view it on GitHub https://github.com/Mu2e/Offline/pull/1288#pullrequestreview-2124257612, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABAH57YLXM47ZB27OY7K2ZLZH6LWBAVCNFSM6AAAAABJPAKNG6VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDCMRUGI2TONRRGI . You are receiving this because you were mentioned.Message ID: @.***>

[FNALbuild]

:memo: The HEAD of main has changed to f397c4a76126622d2eac876b618dd67029a51d3e. Tests are now out of date.

[kutschke]

This change is downstream of reco, it just adds options and improves accès for TrkAna and REve

OK - decide what you want to do about my suggestion about returning a struct rather than returning via arguments. Once that's resolved I will merge.

[brownd1978]

A struct is overly-complicated for this use case. Most callers will just want the t0 value.

On Tue, Jun 18, 2024 at 8:26 AM Rob Kutschke @.***> wrote:

This change is downstream of reco, it just adds options and improves accès for TrkAna and REve

OK - decide what you want to do about my suggestion about returning a struct rather than returning via arguments. Once that's resolved I will merge.

— Reply to this email directly, view it on GitHub https://github.com/Mu2e/Offline/pull/1288#issuecomment-2176380011, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABAH577KSROTCLQ5R2PJOJTZIBGQRAVCNFSM6AAAAABJPAKNG6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNZWGM4DAMBRGE . 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

[kutschke]

@FNALbuild run build test

[FNALbuild]

:hourglass: The following tests have been triggered for bc9a12d7bec460c8638c88e33034a78592061b6f: build (Build queue is empty)

[FNALbuild]

:sunny: The build tests passed at bc9a12d7bec460c8638c88e33034a78592061b6f.

Test	Result	Details
test with	:white_check_mark:	Command did not list any other PRs to include
merge	:white_check_mark:	Merged bc9a12d7bec460c8638c88e33034a78592061b6f at f397c4a76126622d2eac876b618dd67029a51d3e
build (prof)	:white_check_mark:	Log file. Build time: 04 min 09 sec
ceSimReco	:white_check_mark:	Log file.
g4test_03MT	:white_check_mark:	Log file.
transportOnly	:white_check_mark:	Log file.
POT	:white_check_mark:	Log file.
g4study	:white_check_mark:	Log file.
cosmicSimReco	:white_check_mark:	Log file.
cosmicOffSpill	:white_check_mark:	Log file.
ceSteps	:white_check_mark:	Log file.
ceDigi	:white_check_mark:	Log file.
muDauSteps	:white_check_mark:	Log file.
ceMix	:white_check_mark:	Log file.
rootOverlaps	:white_check_mark:	Log file.
g4surfaceCheck	:white_check_mark:	Log file.
FIXME, TODO	:large_orange_diamond:	TODO (0) FIXME (5) in 5 files
clang-tidy	:large_orange_diamond:	0 errors 259 warnings
whitespace check	:white_check_mark:	no whitespace errors found

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

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.

[kutschke]

In the medium term I would like us to move away from return arguments. But that's for another day.