fix: continue-on-error for 2DStrip

[wdconinc]

Briefly, what does this PR introduce? Please link to any relevant presentations or discussions.

This pull request updates the workflow configuration in .github/workflows/linux-eic-shell.yml to improve control over job error handling in the eicrecon-dis job. The main changes introduce a continue-on-error setting to the job matrix, allowing certain job runs (2DStrip in particular) to proceed even if errors occur.

Workflow configuration improvements:

• Added a continue-on-error parameter to the eicrecon-dis job, making it configurable per matrix entry.
• Updated the job matrix to include continue-on-error: [false] by default, and set continue-on-error: true for the configuration using sanitizer: ASAN, allowing that job to continue on error. [1] [2]

What is the urgency of this PR?

• High (please describe reason below)
• Medium
• Low

In Acts v46, the 2DStrip geometry is not compatible with the existing non-2DStrip material map anymore (I.e. it probably never was compatible but now it's a hard failure).

What kind of change does this PR introduce?

• Bug fix (issue #__)
• New feature (issue #__)
• Optimization (issue #__)
• Updated parameters, constants (issue #__)
• Updated documentation
• other: __

Please check if any of the following apply

• This PR requires changes to geometry (epic PR: __)
• This PR requires changes to EDM4eic (EDM PR: __)
• This PR introduces breaking changes. Please describe changes users need to make below.
• This PR changes default behavior. Please describe changes below.
• AI was used in preparing this PR. Please describe usage below.

[Copilot]

Pull request overview

This PR updates the eicrecon-dis GitHub Actions job to allow specific matrix entries (notably the 2DStrip configuration) to proceed without failing the overall workflow by making continue-on-error configurable per matrix entry.

Changes:

• Adds a job-level continue-on-error driven by a matrix value.
• Introduces a matrix variable intended to default continue-on-error to false.
• Marks the 2DStrip+ASAN matrix entry as continue-on-error: true.

Suppressed comments (1)

.github/workflows/linux-eic-shell.yml:1158

• Adding continue-on-error: [false] as a matrix axis will generate an extra matrix job that only has that variable set (and is missing required matrix keys like CXX, beam, etc.), which will likely fail. Since this job uses matrix.include to define the real runs, remove the axis and rely on the per-include override (plus a default in the job expression).

      matrix:
        continue-on-error: [false]
        include:

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[wdconinc]

In Acts v46, the 2DStrip geometry is not compatible with the existing non-2DStrip material map anymore (I.e. it probably never was compatible but now it's a hard failure).

FYI @ShujieL @ybedfer. See for example https://github.com/eic/EICrecon/actions/runs/32655851820/job/97236980791 but also reproducible in eic-shell.

[ybedfer]

Hello,

  [] [acts] [info] CONV           Translating DD4hep sub detector: OuterBarrelMPGDSubAssembly
  [] [acts] [info] CONV           Translating DD4hep sub detector: B0TrackerSubAssembly
  Error: [] [acts] [error] Error during DD4Hep -> ACTS geometry conversion: CylinderSurface::checkSurfaceMaterial: material axis directions {AxisPhi, AxisR} are not supported by this surface. Supported axes are: {AxisRPhi, AxisZ} or {AxisPhi, AxisZ}
  [] [acts] [info] Set parameter acts::InitLogLevel=trace to see conversion info and possibly identify failing geometry

Is this the error that you are referring to? (it's an excerpt from your example "https://github.com/eic/EICrecon/actions/runs/32655851820/job/97236980791", line 299).
If so, how can it be that 2DStrip triggers the problem? As far as I can see, the culprit is rather B0TrackerSubAssembly, isn'it? In addition, 2DStrip'd detector have no CylinderSurface.

I have to admit that I don't understand much of the log that you are pointing to. Can you explain to me how you get to the conclusion that it's a 2DStrip issue? Is it merely because when you do not use a 2DStrip configuration, the problem does not show up any more?

[wdconinc]

Hello,

  [] [acts] [info] CONV           Translating DD4hep sub detector: OuterBarrelMPGDSubAssembly
  [] [acts] [info] CONV           Translating DD4hep sub detector: B0TrackerSubAssembly
  Error: [] [acts] [error] Error during DD4Hep -> ACTS geometry conversion: CylinderSurface::checkSurfaceMaterial: material axis directions {AxisPhi, AxisR} are not supported by this surface. Supported axes are: {AxisRPhi, AxisZ} or {AxisPhi, AxisZ}
  [] [acts] [info] Set parameter acts::InitLogLevel=trace to see conversion info and possibly identify failing geometry

Is this the error that you are referring to? (it's an excerpt from your example "https://github.com/eic/EICrecon/actions/runs/32655851820/job/97236980791", line 299).
If so, how can it be that 2DStrip triggers the problem? As far as I can see, the culprit is rather B0TrackerSubAssembly, isn'it? In addition, 2DStrip'd detector have no CylinderSurface.

The B0 is in all other jobs, but only the 2DStrip job fails.

I have to admit that I don't understand much of the log that you are pointing to. Can you explain to me how you get to the conclusion that it's a 2DStrip issue? Is it merely because when you do not use a 2DStrip configuration, the problem does not show up any more?

If you run EICrecon with verbose or trace log level on the acts plugin you get more info on what Acts is doing when it fails. It fails when handling the mpgd barrel.

[ybedfer]

If you run EICrecon with verbose or trace log level on the acts plugin you get more info on what Acts is doing when it fails. It fails when handling the mpgd barrel.

I have run EICrecon many times w/ a 2DStrip configuration w/o any problem. Is the present problem only showing up when the newest HEAD version of the git distribution (of EICrecon) is used?

[wdconinc]

If you run EICrecon with verbose or trace log level on the acts plugin you get more info on what Acts is doing when it fails. It fails when handling the mpgd barrel.

I have run EICrecon many times w/ a 2DStrip configuration w/o any problem. Is the present problem only showing up when the newest HEAD version of the git distribution (of EICrecon) is used?

It happens with the upgraded Acts version v46.

[wdconinc]

It happens with the upgraded Acts version v46.

We run tests in new Acts versions before upgrades, but we run this with a focus on the full standard geometry, not alternatives. So that's why this wasn't noticed earlier.

[ybedfer]

It happens with the upgraded Acts version v46.

To be sure that I am understanding well: if I now run EICrecon w/ a 2DStrip configuration w/in current eic-shell, I will get the problem?

[wdconinc]

It happens with the upgraded Acts version v46.

To be sure that I am understanding well: if I now run EICrecon w/ a 2DStrip configuration w/in current eic-shell, I will get the problem?

Yes, that's correct. If you run with the currently active material map and with the geometry of the failing job you will get the problem. I was able to reproduce this easily by just copying the commands from the failing job.

[ybedfer]

run with the currently active material map

And could it simply be that 2DStrip configurations need (at least now that we have Acts,v46) their own material map?
How can I test this hypothesis? Where can I find the instructions to build a material map (which I guess must be compiled for a specific $DETECTOR_CONFIG)? Then I could try to generate a dedicated 2DStrip map...

[wdconinc]

run with the currently active material map

And could it simply be that 2DStrip configurations need (at least now that we have Acts,v46) their own material map? How can I test this hypothesis? Where can I find the instructions to build a material map (which I guess must be compiled for a specific $DETECTOR_CONFIG)? Then I could try to generate a dedicated 2DStrip map...

In the epic repository there is a directory scripts/ with material map scripts. If we need to regenerate materials maps for any geometry where only the readout definition changes then this is a bit surprising to me and I'd like to understand why. Bit I agree that that would be a way to make it work.

[ybedfer]

If we need to regenerate materials maps for any geometry where only the readout definition changes

The pixel configurations ("pixel" as opposed to "2DStrip") are only temporary.

And then 2DStrip is slightly more complicated than a change in the readout. I subdivide the sensitive volume into several subvolumes, all w/ an attached sensitive surface. (Btw, I have always been wondering about the double thickness_inner , double thickness_outer arguments to the DD4hep::DDRecVolPlane constructor: for a unique sensitive volume, the instructions to set these are straightforward enough, but in my multiple subvolumes case, I am not sure. Could that be the origin of the problem?)

[wdconinc]

In the epic repository there is a directory scripts/ with material map scripts. If we need to regenerate materials maps for any geometry where only the readout definition changes then this is a bit surprising to me and I'd like to understand why. Bit I agree that that would be a way to make it work.

There are some other steps that need to succeed before we can recreate a material map though:

• hotfix: spack cherry-pick: acts: pass root variant to examples containers#397
• fix: update material map validation script to Acts v46.8.1 epic#1159

[ShujieL]

Thank you @wdconinc for this notice.
If I read the error message at face value:

Error: [] [acts] [error] Error during DD4Hep -> ACTS geometry conversion: CylinderSurface::checkSurfaceMaterial: material axis directions {AxisPhi, AxisR} are not supported by this surface. Supported axes are: {AxisRPhi, AxisZ} or {AxisPhi, AxisZ}

It reads a phi-R binning scheme, but expects Rphi-Z, or phi-Z for cylinder. @ybedfer can you identify relevant code in the 2dstrip description? Probably update the surface type to disk (to go with phi-R binning) or change the binning to phi-Z (that works with cylinder) can fix the issue?

[ybedfer]

@wdconinc

There are some other steps that need to succeed before we can recreate a material map though:

* [ ]  [hotfix: spack cherry-pick: acts: pass root variant to examples containers#397](https://github.com/eic/containers/pull/397)

* [ ]  [fix: update material map validation script to Acts v46.8.1 epic#1159](https://github.com/eic/epic/pull/1159)

Hello,
When I execute the run_material_map_validation.sh script, I get the following error:

  File "/local/home/bedfer/eicA/epicMain/scripts/material_map/material_recording_epic.py", line 22, in <module>
    from material_recording import runMaterialRecording
  File "/local/home/bedfer/eicA/epicMain/scripts/material_map/Examples/Scripts/Python/material_recording.py", line 20, in <module>
    from acts.examples.root import RootMaterialTrackWriter
  File "/opt/local/python/acts/examples/root.py", line 5, in <module>
    from .ActsExamplesPythonBindingsRoot import *
ModuleNotFoundError: No module named 'acts.examples.ActsExamplesPythonBindingsRoot'

Is this connected to the two issues you mention supra? Or is it yet another problem?

[wdconinc]

Is this connected to the two issues you mention supra? Or is it yet another problem?

That's problem 1.

[ybedfer]

@wdconinc
Hello again,
With a fresh eic-shell the previous error (complaining about lack of ROOT stuff) is gone, but I get another error:

./run_material_map_validation.sh --nevents 1000 --nparticles 5000
::group::----GEANTINO SCAN------
PluginService    ERROR Factory requested: epic_FileLoader (N10__cxxabiv120__function_type_infoE) :bad any_cast
PluginService    ERROR Stub is invalid!
Traceback (most recent call last):
  File "/local/home/bedfer/eicA/epicMain/scripts/material_map/material_recording_epic.py", line 69, in <module>
    main()
    ~~~~^^
  File "/local/home/bedfer/eicA/epicMain/scripts/material_map/material_recording_epic.py", line 56, in main
    detector = epic.getDetector(
        args.xmlFile)
  File "/local/home/bedfer/eicA/epicMain/scripts/material_map/epic.py", line 37, in getDetector
    detector = acts.examples.dd4hep.DD4hepDetector(dd4hepConfig)
  File "/opt/local/python/acts/_adapter.py", line 14, in wrapped
    fn(self, maybe_config, *args[1:], **kwargs)
    ~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
RuntimeError: dd4hep: apply-plugin: Failed to locate plugin epic_FileLoader. 	No factory with name Create(epic_FileLoader) for type epic_FileLoader found.
		Please check library load path and/or plugin factory name.
dd4hep: with plugin:epic_FileLoader
dd4hep: Error interpreting XML nodes of type <plugin/>
dd4hep: Error interpreting XML nodes of type <plugins/>
dd4hep: Error interpreting XML nodes of type <include/>
dd4hep: while parsing /opt/software/linux-x86_64_v2/epic-git.9aaa296976d3ad9de404f775ae89fc17a068c07c_main-6ssrmjmjdcwyhfagapr4bxeljijwsobx/share/epic/epic_craterlake_material_map.xml

Is this a known issue?

[wdconinc]

You should first run source /opt/detector/epic-main/bin/thisepic.sh. The script assumes we are running inside an environment where that is defined.

[ybedfer]

Sorry, that was obvious.

Now, the execution runs smoothly... but I don't get any .cbor file output. I have a ./material-map_map.json but no .cbor.

[wdconinc]

Sorry, that was obvious.

Now, the execution runs smoothly... but I don't get any .cbor file output. I have a ./material-map_map.json but no .cbor.

Yes, we lost cbor support along the way. I'm working on a fix in acts for that. EICrecon should be able to use json as well. Only problem is it's 400 MB.

[ybedfer]

Hello again,

With my newly produced material map:

19:48:47.420  [info]  - key:         acts:MaterialMap
19:48:47.420  [info]    value:       /local/home/bedfer/eicA/epicMain/scripts/material_map/material-map_map.json
19:48:47.420  [info]    default:     calibrations/materials-map.cbor
19:48:47.420  [info]    description: JSON/CBOR material map file path

I still get the error:

[] [acts] [error] Error during DD4Hep -> ACTS geometry conversion: CylinderSurface::checkSurfaceMaterial: material axis directions {AxisPhi, AxisR} are not supported by this surface. Supported axes are: {AxisRPhi, AxisZ} or {AxisPhi, AxisZ}

And then:

[e:0] [JEventProcessorPODIO] [error] Omitting PODIO collection 'B0TrackerCKFTrackAssociations' due to exception: JOmniFactory: Failed to get collection B0TrackerMeasurements: CylinderSurface::checkSurfaceMaterial: material axis directions {AxisPhi, AxisR} are not supported by this surface. Supported axes are: {AxisRPhi, AxisZ} or {AxisPhi, AxisZ}.

I then don't know what to do....

[wdconinc]

That's interesting. So even a regenerated material map still fails, eh. Hmm... This will require more inspection then.

The next step is likely to run with -Pacts:LogLevel=trace or so, and look at what EICrecon prints out. It would help to identify which layer/volume is responsible exactly. Then we can go into the material map and more directly figure out why it fails to encode the material map for that layer/volume correctly.

[veprbl]

Is this material map or surface issue? I think surfaces define their coordinate systems. We could specify a different value via an additional entry in DD4hep_ParametersPlugin.

[ybedfer]

Hello again,
I hadn't included the ecals in my 2DStrip configuration.
When I do include them:

diff --git a/configurations/craterlake_tracking_2DStrip.yml b/configurations/craterlake_tracking_2DStrip.yml
index bb8e21b63..6ba0412a3 100644
--- a/configurations/craterlake_tracking_2DStrip.yml
+++ b/configurations/craterlake_tracking_2DStrip.yml
@@ -17,0 +18,2 @@ features:
+  ecal:
+    bic_layer1_only:

I have no error any more!
Could it be that simple? Why would it make a difference (whether ecals or not)?

=> Anyway I will prepare a PR of epic w/ the above modification of configuration and let you check.

=> Next issue: would we need a dedicated 2DStrip material map, still? It turns out that the trace-level log of eicrecon initialization differs between pixel and 2DStrip configurations. In particular, regarding the # of surfaces created: 32 (= # of modules) against 160 (= 32 × # of sensitive subvolumes). This, for CyMBal. For the OuterBarrelMPGD: 24 against 120. There are also a few numerics that differ here and there.