Introduce SurfaceStep

[brownd1978]

SurfaceStep is a new MC truth class that functions like StrawGasStep, CaloShowerStep, etc, but for passive materials and virtual detectors. It uses the SurfaceId class to define the surfaces, and summarizes the information contained in contiguous StepPointMC objects. This greatly reduced the payload and renders the output information more useful for physics studies, and insulates the results from the details of the G4 stepping parameters. This PR introduces the class itself plus producer, diag module, printer, and default production configuration.

Note that SurfaceId was moved to DataProducts (from RecoDataProducts and KinKalGeom) to allow this to work. It's fine, as the surface concept isn't specific to reco or mc.

[FNALbuild]

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

• CommonMC
• RecoDataProducts
• Print
• DataProducts
• MCDataProducts
• TrackerMC
• Mu2eKinKal
• KinKalGeom
• Validation

which require these tests: build.

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

⌛ The following tests have been triggered for 680a6ef: build (Build queue has 1 jobs)

About FNALbuild. Code review on Mu2e/Offline.

[FNALbuild]

☀️ The build tests passed at 680a6ef.

Test	Result	Details
test with	✅	Command did not list any other PRs to include
merge	✅	Merged 680a6ef at cecf5be
build (prof)	✅	Log file. Build time: 08 min 24 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 (21) FIXME (2) in 15 files
clang-tidy	🔶	0 errors 580 warnings
whitespace check	✅	no whitespace errors found

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

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.

[rlcee]

• "surface" would be more clear as "material" or "volume"
• split SurfaceId from enum
• SurfaceId::index could be named more clearly and commented
• fix include guards in SurfaceId
• print looks fine

[brownd1978]

• "surface" would be more clear as "material" or "volume"
  Surface is explicit in the downstream use cases, particularly reconstruction. 'volume' or 'material' are less accurate.
• split SurfaceId from enum
  These classes don't make sense as independent objects, just like the detail of the enum class it is clearer to keep them in 1 file
• SurfaceId::index could be named more clearly and commented
  I added some comments. The name must be general.
• fix include guards in SurfaceId
  done
• print looks fine

Note too SurfaceId pre-exists this PR by over a year, this PR simply moves it.

[kutschke]

I saw comment a comment in the conversation (or an issue?) that explains that index class distinguishes items with multiple placements, such as the target foils. I don't see it explained in any of the code files. Did I miss it? If not, please add a comment to SurfaceId.hh . Also, are there other entities that have multiple indices?

[kutschke]

I know that I started this incorrectly a long time ago. I recommend that the declartion go here and the implementation in the .cc file.

[kutschke]

Thanks for making the changes.

[kutschke]

@sdifalco a gentle reminder that this PR is on your to-review list.

[kutschke]

@FNALbuild run build test

[FNALbuild]

⌛ The following tests have been triggered for 491ff6c: build (Build queue has 1 jobs)

[FNALbuild]

☔ The build tests failed for 491ff6c.

Test	Result	Details
test with	✅	Command did not list any other PRs to include
merge	✅	Merged 491ff6c at 616f60b
build (prof)	✅	Log file. Build time: 04 min 10 sec
ceSimReco	❌	Log file. Return Code 90.
g4test_03MT	✅	Log file.
transportOnly	✅	Log file.
POT	✅	Log file.
g4study	✅	Log file.
cosmicSimReco	❌	Log file. Return Code 90.
cosmicOffSpill	❌	Log file. Return Code 90.
ceSteps	❌	Log file. Return Code 90.
ceDigi	❌	Log file. Return Code 90.
muDauSteps	❌	Log file. Return Code 90.
ceMix	❌	Log file. Return Code 90.
rootOverlaps	✅	Log file.
g4surfaceCheck	✅	Log file.
FIXME, TODO	🔶	TODO (21) FIXME (2) in 15 files
clang-tidy	🔶	0 errors 614 warnings
whitespace check	✅	no whitespace errors found

N.B. These results were obtained from a build of this Pull Request at 491ff6c after being merged into the base branch at 616f60b.

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]

Needs to test with a Production PR

@FNALbuild run build test with Mu2e/Production#329

[FNALbuild]

⌛ The following tests have been triggered for 491ff6c: build (Build queue has 1 jobs)

[FNALbuild]

☀️ The build tests passed at 491ff6c.

Test	Result	Details
test with	✅	Included Mu2e/Production#329 @ 5287648aedc1c528dafe514365f75b6a1a4f8e83 by merge
merge	✅	Merged 491ff6c at 616f60b
build (prof)	✅	Log file. Build time: 08 min 22 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 (21) FIXME (2) in 15 files
clang-tidy	🔶	0 errors 614 warnings
whitespace check	✅	no whitespace errors found

N.B. These results were obtained from a build of this Pull Request at 491ff6c after being merged into the base branch at 616f60b.

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]

AFAIK I've addressed all comments and have no further plans to work on this. Stefano was added as reviewer just to keep him in the loop.

[FNALbuild]

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

[sdifalco]

@sdifalco a gentle reminder that this PR is on your to-review list.

Sorry but I am been (and still am) very busy with funding requests to INFN in these days. Thanks for taking me in the loop. I will approve what you have already commented. Thanks Ed for the nice job!

[kutschke]

AFAIK I've addressed all comments and have no further plans to work on this. Stefano was added as reviewer just to keep him in the loop.

In that case it's ready to merge. Do you expect this to have impact on the nightly validation? The STM PR that I merged earlier does modify the G4 geometry but only in the STM region.

[kutschke]

I will go ahead and merge now under the assumption that if there are issues to sort out with the nightly, we can do it tomoorow