Add flexibility to add configuration changes for Run 1B targets

[michaelmackenzie]

This allows us to add arbitrary tubes to the DS2 volume from the TSdA construction. This also allows for off-centered and partial holes in the TSdA when used as a target plate. The tsda.extra. tube is an early iteration, still needed for backwards compatibility with previous configuration tests.

[FNALbuild]

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

• Mu2eG4

which require these tests: build.

@Mu2e/write have access to CI actions on Run1B.

⌛ The following tests have been triggered for 0f4618a: build (Build queue - API unavailable)

About FNALbuild. Code review on Mu2e/Offline.

[oksuzian]

I found 2 likely issues in this PR.

Assessment: Not ready to merge yet — the change is small and focused, but there are two compatibility/correctness risks in Mu2eG4/src/constructTSdA.cc. CI status is currently unclear from GitHub’s check API (mergeable_state: unstable, no check runs returned yet, and FNALbuild says build was queued).

PR summary

• PR: #1827 — Add flexibility to add configuration changes for Run 1B targets
• Author: michaelmackenzie
• Base → Head: Run1B ← Run1BTargetDesigns
• Scope: 1 file changed, +120 / -16
• Reviews: none yet
• Comments: 1 bot comment
• Mergeable: yes, but state is unstable
• Risk: medium, because this changes geometry construction and config parsing behavior

Main issues

1. std::format may break the build on supported compiler/toolchain combinations

   • The PR adds:

     #include <format>
     ...
     nestTubs(std::format("TSdATube{}", i), ...)

   • In many HEP/CentOS-style toolchains, full std::format support is still spotty even when other C++20 features work.
   • This is especially risky in infrastructure code that previously did not require <format>.
   • Safer alternatives:
     • std::string("TSdATube") + std::to_string(i)
     • or a small stringstream

   Relevant lines:
   constructTSdA.cc:47, constructTSdA.cc:227

   #include <format>

   for(size_t i = 0; i < n_tubes; ++i) {
     ...
     nestTubs(std::format("TSdATube{}", i),
              params,
              material,
              0,
              offset,
              ds2VacuumInfo,
              0,
              NAVisible,
              G4Colour::Cyan(),
              NASolid,
              forceAuxEdgeVisible,
              placePV,
              doSurfaceCheck
              );
   }

2. Changing tsda.rin default semantics may silently alter existing configs that intentionally set it to 0

   • Old behavior:
     • tmpRin = getDouble("tsda.rin", 0.0)
     • if tmpRin < 1e-6, use TS5 default radius
     • so unset or zero both meant “use default”
   • New behavior:
     • tmpRin = getDouble("tsda.rin", -1.0)
     • if tmpRin < 0, use default
     • so explicit 0 now means real inner radius = 0
   • That is a behavioral change, not just an implementation cleanup.
   • If any existing configs relied on tsda.rin = 0 as shorthand for “default”, this PR will change geometry unexpectedly.

   The PR description mentions backward compatibility for tsda.extra.*, but not for this tsda.rin semantic change.

   Relevant lines:
   constructTSdA.cc:56, constructTSdA.cc:78

   int const verbosityLevel = _config.getInt("tsda.verbosityLevel",0);
   double tmpRin = _config.getDouble("tsda.rin",-1.0)*CLHEP::mm;

   int version = atsd->version();
   if ( tmpRin < 0. ) {  // check for negative value indicating to use the default value
     GeomHandle<Beamline> beamg;
     const StraightSection * ts5out = beamg->getTS().getTSCryo<StraightSection>( TransportSolenoid::TSRegion::TS5,
                                                                                 TransportSolenoid::TSRadialPart::IN );
     tmpRin = ts5out->rIn();
   }

Other observations

• The cutout feature using G4SubtractionSolid looks reasonable.
• The vector-size validation for tsda.tubes.* is good.
• No human review comments yet, so these are fresh findings rather than duplicates.

Suggested review comments

You could leave comments like:

1. On std::format:
   “Using <format> here may not be portable across our build toolchains. Could we use std::to_string(i) concatenation instead to avoid introducing a C++20 library dependency?”

2. On tsda.rin:
   “This changes the meaning of tsda.rin = 0 from ‘use default TS5 radius’ to ‘use an actual zero inner radius’. Is that intentional, and do we know existing configs won’t rely on the previous behavior?”

Want me to

1. Draft exact GitHub review comments you can paste
2. Do a deeper geometry/physics correctness pass on this file
3. Re-check CI once build results appear

[FNALbuild]

☀️ The build tests passed at 0f4618a.

Test	Result	Details
test with	✅	Command did not list any other PRs to include
merge	✅	Merged 0f4618a at 5ec862f
build (prof)	✅	Log file. Build time: 04 min 09 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.
trigger	❌	Log file. Return Code 2.
check_cmake	✅	Log file.
FIXME, TODO	✅	TODO (0) FIXME (0) in 0 files
clang-tidy	✅	0 errors 0 warnings
whitespace check	✅	no whitespace errors found

N.B. These results were obtained from a build of this Pull Request at 0f4618a after being merged into the base branch at 5ec862f.

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.

[michaelmackenzie]

Responding to the AI comments:

1. I changed from std::format --> std::to_string
2. I checked that no geom files in the repo using tsda.rin = 0, so this shouldn't impact past geometries

[oksuzian]

@FNALbuild run build test

[FNALbuild]

⌛ The following tests have been triggered for d5ae629: build (Build queue - API unavailable)

[FNALbuild]

☀️ The build tests passed at d5ae629.

Test	Result	Details
test with	✅	Command did not list any other PRs to include
merge	✅	Merged d5ae629 at 5ec862f
build (prof)	✅	Log file. Build time: 08 min 35 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.
trigger	❌	Log file. Return Code 2.
check_cmake	✅	Log file.
FIXME, TODO	✅	TODO (0) FIXME (0) in 0 files
clang-tidy	✅	0 errors 0 warnings
whitespace check	✅	no whitespace errors found

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

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.