Mu2e / Offline

Offline software for the Mu2e experiment
Apache License 2.0
8 stars 81 forks source link

val track charge fix #1255

Closed rlcee closed 5 months ago

rlcee commented 5 months ago

I found that some validation jobs were ended with a "tried to save duplicate histogram" error, which I fixed with adding the "2". Then I reran my test and it seg-faulted on this line double ksCharge = ks.intersections().front().pstate_.charge(); I see that the intersections access is protected against empty vectors in the other access, and if an empty vector is possible here, then this line could cause undefined behavior and I got lucky. So I moved it into the protected block. But I also lazily switched to the charge of the sid surface intersection instead of the front of the intersections vector. I suspect this is OK for this purpose (an more consistent?), but if the front of the vector is important, then I can re-write to get that safely.

FNALbuild commented 5 months ago

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

which require these tests: build.

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

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

About FNALbuild. Code review on Mu2e/Offline.

FNALbuild commented 5 months ago

:sunny: The build tests passed at 90f7387abc836058d5fb1670a13e4cd669cc3f23.

Test Result Details
test with :white_check_mark: Command did not list any other PRs to include
merge :white_check_mark: Merged 90f7387abc836058d5fb1670a13e4cd669cc3f23 at 07a76f5fcecbc4e3ef9c6edf73c17dd777164d46
build (prof) :white_check_mark: Log file. Build time: 21 min 19 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 (1) FIXME (0) in 1 files
clang-tidy :large_orange_diamond: 0 errors 239 warnings
whitespace check :white_check_mark: no whitespace errors found

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

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 commented 5 months ago

This is superseded by PR #1257. Closing it.

kutschke commented 5 months ago

I have provided an alternate, more robust and definitive fix in PR 1258. Please use that instead.

Yes - I saw it.