feat: move ROOT surface material map I/O to plugins by asalzburger · Pull Request #4400 · acts-project/acts

asalzburger · 2025-07-08T14:06:31Z

This PR moves the I/O for material map reading/writing into Plugins/Root and removes the duplicated code from RootMaterialWriter and RootMaterialDecorator.

It also harmonizes the usage/definition of SurfaceMaterialMaps. VolumeMaterialMaps, and DetectorMaterialMaps consistently by declaring them once.

It is backward compatible, and introduces a new, common ROOT I/O material backend that can be shared between all material classes (homongeneous, binned, grid, indexed).

This PR also allows UnitTests to be added for Material writing, the first ones are done with it, and it removes roughly 100 SonarCloud issues, mainly related to how we call and use ROOT.

This will be followed by a PRs doing the same for Volume material and updated Grid based surface material.

The reading gis kept backward-compatible.

--- END COMMIT MESSAGE ---

Any further description goes here, @-mentions are ok here!

Use a conventional commits prefix: quick summary
- We mostly use feat, fix, refactor, docs, chore and build types.
A milestone will be assigned by one of the maintainers

github-actions · 2025-07-08T17:59:05Z

📊: Physics performance monitoring for `c94bc8a`

Full contents

physmon summary

paulgessinger

I like the approach, but have a few comments on the implementation.

Core/include/Acts/Material/DetectorMaterialMaps.hpp

...Algorithms/MaterialMapping/include/ActsExamples/MaterialMapping/MappingMaterialDecorator.hpp

Examples/Io/Root/include/ActsExamples/Io/Root/RootMaterialDecorator.hpp

Examples/Io/Root/src/RootMaterialDecorator.cpp

Plugins/Json/include/Acts/Plugins/Json/JsonMaterialDecorator.hpp

Plugins/Root/include/Acts/Plugins/Root/RootMaterialMapAccessor.hpp

Plugins/Root/src/RootMaterialMapAccessor.cpp

Tests/UnitTests/Plugins/Root/RootMaterialMapAccessorTests.cpp

asalzburger · 2025-07-12T05:28:06Z

Mac OsX did strike again - the combination of capital / small letters and git is horrendous when removing.

I just changed "IO" to "Io" in the tests, so I suppose the "approval" is still valid.

sonarqubecloud · 2025-07-12T06:39:12Z

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
40.6% Coverage on New Code
0.7% Duplication on New Code

See analysis details on SonarQube Cloud

paulgessinger · 2025-07-13T19:56:27Z

This breaks the Athena build. I'm reverting this until we have a fix in #4424

Failing job: https://gitlab.cern.ch/acts/acts-athena-ci/-/jobs/58703357#L1885

In file included from /builds/acts/acts-athena-ci/athena/Tracking/Acts/ActsGeometry/ActsGeometry/ActsMaterialJsonWriterTool.h:16,
                 from /builds/acts/acts-athena-ci/athena/Tracking/Acts/ActsGeometry/src/components/ActsGeometry_entries.cxx:11:
/builds/acts/acts-athena-ci/athena/Tracking/Acts/ActsGeometryInterfaces/ActsGeometryInterfaces/IActsMaterialJsonWriterTool.h:24:80: error: 'DetectorMaterialMaps' in 'class Acts::MaterialMapJsonConverter' does not name a type
   24 |   write(const ActsGeometryContext& gctx, const Acts::MaterialMapJsonConverter::DetectorMaterialMaps& detMaterial) const = 0;
      |                                                                                ^~~~~~~~~~~~~~~~~~~~
/builds/acts/acts-athena-ci/athena/Tracking/Acts/ActsGeometry/ActsGeometry/ActsMaterialJsonWriterTool.h:39:80: error: 'DetectorMaterialMaps' in 'class Acts::MaterialMapJsonConverter' does not name a type
   39 |   write(const ActsGeometryContext& gctx, const Acts::MaterialMapJsonConverter::DetectorMaterialMaps& detMaterial) const override;
      |                                                                                ^~~~~~~~~~~~~~~~~~~~
[908/1143] Building CXX object Tracking/Acts/ActsGeometry/CMakeFiles/ActsGeometryLib.dir/src/ActsTrackingGeometrySvc.cxx.o

acts-project#4400] This reverts commit d036765.

#4400] (#4424) This reverts commit d036765.

asalzburger · 2025-07-13T20:09:48Z

Interesting ... do we have a dependency in Athena on ActsExamples?

asalzburger · 2025-07-13T20:11:19Z

Ah no, it was the consistent naming as we use the Plugins/Json for reading the map.

paulgessinger · 2025-07-14T05:43:29Z

We could reintroduce the old aliases as deprecated for the moment, or mark this breaking and prepare a patch.

This PR moves the I/O for material map reading/writing into `Plugins/Root` and removes the duplicated code from `RootMaterialWriter` and `RootMaterialDecorator`. It also harmonizes the usage/definition of `SurfaceMaterialMaps`. `VolumeMaterialMaps`, and `DetectorMaterialMaps` consistently by declaring them once. It is backward compatible, and introduces a new, common ROOT I/O material backend that can be shared between all material classes (homongeneous, binned, grid, indexed). This PR also allows UnitTests to be added for Material writing, the first ones are done with it, and it removes roughly 100 SonarCloud issues, mainly related to how we call and use ROOT. This will be followed by a PRs doing the same for Volume material and updated Grid based surface material. The reading gis kept backward-compatible. --- END COMMIT MESSAGE --- Any further description goes here, @-mentions are ok here! - Use a *conventional commits* prefix: [quick summary](https://www.conventionalcommits.org/en/v1.0.0/#summary) - We mostly use `feat`, `fix`, `refactor`, `docs`, `chore` and `build` types. - A milestone will be assigned by one of the maintainers --------- Co-authored-by: Paul Gessinger <hello@paulgessinger.com> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>

acts-project#4400] (acts-project#4424) This reverts commit d036765.

This PR moves the I/O for material map reading/writing into `Plugins/Root` and removes the duplicated code from `RootMaterialWriter` and `RootMaterialDecorator`. It also harmonizes the usage/definition of `SurfaceMaterialMaps`. `VolumeMaterialMaps`, and `DetectorMaterialMaps` consistently by declaring them once. It is backward compatible, and introduces a new, common ROOT I/O material backend that can be shared between all material classes (homongeneous, binned, grid, indexed). This PR also allows UnitTests to be added for Material writing, the first ones are done with it, and it removes roughly 100 SonarCloud issues, mainly related to how we call and use ROOT. This will be followed by a PRs doing the same for Volume material and updated Grid based surface material. The reading gis kept backward-compatible. --- END COMMIT MESSAGE --- Any further description goes here, @-mentions are ok here! - Use a *conventional commits* prefix: [quick summary](https://www.conventionalcommits.org/en/v1.0.0/#summary) - We mostly use `feat`, `fix`, `refactor`, `docs`, `chore` and `build` types. - A milestone will be assigned by one of the maintainers --------- Co-authored-by: Paul Gessinger <hello@paulgessinger.com> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>

acts-project#4400] (acts-project#4424) This reverts commit d036765.

Reintroduction of #4400 with the previous typedefs added back in as deprecated. Co-authored-by: Andreas Salzburger <asalzburger@gmail.com>

This PR moves the I/O for material map reading/writing into `Plugins/Root` and removes the duplicated code from `RootMaterialWriter` and `RootMaterialDecorator`. It also harmonizes the usage/definition of `SurfaceMaterialMaps`. `VolumeMaterialMaps`, and `DetectorMaterialMaps` consistently by declaring them once. It is backward compatible, and introduces a new, common ROOT I/O material backend that can be shared between all material classes (homongeneous, binned, grid, indexed). This PR also allows UnitTests to be added for Material writing, the first ones are done with it, and it removes roughly 100 SonarCloud issues, mainly related to how we call and use ROOT. This will be followed by a PRs doing the same for Volume material and updated Grid based surface material. The reading gis kept backward-compatible. --- END COMMIT MESSAGE --- Any further description goes here, @-mentions are ok here! - Use a *conventional commits* prefix: [quick summary](https://www.conventionalcommits.org/en/v1.0.0/#summary) - We mostly use `feat`, `fix`, `refactor`, `docs`, `chore` and `build` types. - A milestone will be assigned by one of the maintainers --------- Co-authored-by: Paul Gessinger <hello@paulgessinger.com> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>

acts-project#4400] (acts-project#4424) This reverts commit d036765.

Reintroduction of acts-project#4400 with the previous typedefs added back in as deprecated. Co-authored-by: Andreas Salzburger <asalzburger@gmail.com>

This PR moves the I/O for material map reading/writing into `Plugins/Root` and removes the duplicated code from `RootMaterialWriter` and `RootMaterialDecorator`. It also harmonizes the usage/definition of `SurfaceMaterialMaps`. `VolumeMaterialMaps`, and `DetectorMaterialMaps` consistently by declaring them once. It is backward compatible, and introduces a new, common ROOT I/O material backend that can be shared between all material classes (homongeneous, binned, grid, indexed). This PR also allows UnitTests to be added for Material writing, the first ones are done with it, and it removes roughly 100 SonarCloud issues, mainly related to how we call and use ROOT. This will be followed by a PRs doing the same for Volume material and updated Grid based surface material. The reading gis kept backward-compatible. --- END COMMIT MESSAGE --- Any further description goes here, @-mentions are ok here! - Use a *conventional commits* prefix: [quick summary](https://www.conventionalcommits.org/en/v1.0.0/#summary) - We mostly use `feat`, `fix`, `refactor`, `docs`, `chore` and `build` types. - A milestone will be assigned by one of the maintainers --------- Co-authored-by: Paul Gessinger <hello@paulgessinger.com> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>

acts-project#4400] (acts-project#4424) This reverts commit d036765.

Reintroduction of acts-project#4400 with the previous typedefs added back in as deprecated. Co-authored-by: Andreas Salzburger <asalzburger@gmail.com>

asalzburger added 6 commits July 7, 2025 12:38

snapshot

20c2350

snapshot

3bf4216

snapshot

ea24b12

next snapshot

c2507c0

cleanup volume maps / surface maps

4423ef6

surface material i/o moved

2242550

asalzburger added this to the next milestone Jul 8, 2025

Merge branch 'main' into chore-move-material-map-io-to-plugins

1f29357

github-actions bot added Component - Core Affects the Core module Component - Examples Affects the Examples module Component - Plugins Affects one or more Plugins labels Jul 8, 2025

asalzburger changed the title ~~feat: move material map io to plugins~~ feat: move surface material map io to plugins Jul 8, 2025

asalzburger added 2 commits July 8, 2025 16:59

Merge branch 'main' into chore-move-material-map-io-to-plugins

9bb6bd4

checking against potential nullptr

4d4233e

asalzburger added 7 commits July 8, 2025 23:01

remove new operator usage

3e2583f

remove SonarCloud issues

ad2c8dd

Merge branch 'main' into chore-move-material-map-io-to-plugins

c8cbdbf

int vs size

a31211e

fixing CI

390e103

make clang-tidy happy

f6c54e2

more SonarCloud issues

7b59369

asalzburger mentioned this pull request Jul 9, 2025

feat: simplify GridSurfaceMaterial classes #4408

Closed

asalzburger added 3 commits July 9, 2025 10:54

more SonarCloud issues

8075386

Merge branch 'main' into chore-move-material-map-io-to-plugins

8f7718c

more SonarCloud issues fixed

3deed01

asalzburger changed the title ~~feat: move surface material map io to plugins~~ feat: move surface material map I/O to plugins Jul 10, 2025

paulgessinger reviewed Jul 10, 2025

View reviewed changes

asalzburger added 2 commits July 10, 2025 09:10

chore-move-material-map-io-to-plugins

a86122c

Merge branch 'main' into chore-move-material-map-io-to-plugins

3af4073

asalzburger force-pushed the chore-move-material-map-io-to-plugins branch from b4e65eb to 899d502 Compare July 12, 2025 04:51

kodiakhq bot and others added 3 commits July 12, 2025 04:51

Merge branch 'main' into chore-move-material-map-io-to-plugins

3a9acce

break

b3a39cc

unbreak

c94bc8a

asalzburger removed the automerge label Jul 12, 2025

asalzburger mentioned this pull request Jul 12, 2025

feat: Add ROOT space point I/O #4398

Merged

asalzburger merged commit d036765 into acts-project:main Jul 12, 2025
38 of 40 checks passed

paulgessinger added a commit to paulgessinger/acts that referenced this pull request Jul 13, 2025

revert: feat: move ROOT surface material map I/O to plugins [originally

c5453c6

acts-project#4400] This reverts commit d036765.

paulgessinger added a commit to paulgessinger/acts that referenced this pull request Jul 13, 2025

revert: feat: move ROOT surface material map I/O to plugins [originally

8607343

acts-project#4400] This reverts commit d036765.

paulgessinger added a commit that referenced this pull request Jul 13, 2025

revert: feat: move ROOT surface material map I/O to plugins [originally

220db58

#4400] (#4424) This reverts commit d036765.

paulgessinger mentioned this pull request Jul 14, 2025

fix: Add material typedefs back with deprecation #4425

Merged

mimodekjaer pushed a commit to mimodekjaer/acts that referenced this pull request Jul 14, 2025

revert: feat: move ROOT surface material map I/O to plugins [originally

44b5b1f

acts-project#4400] (acts-project#4424) This reverts commit d036765.

mimodekjaer pushed a commit to mimodekjaer/acts that referenced this pull request Jul 14, 2025

revert: feat: move ROOT surface material map I/O to plugins [originally

67b8b4c

acts-project#4400] (acts-project#4424) This reverts commit d036765.

kodiakhq bot pushed a commit that referenced this pull request Jul 14, 2025

fix: Add material typedefs back with deprecation (#4425)

f3a918c

Reintroduction of #4400 with the previous typedefs added back in as deprecated. Co-authored-by: Andreas Salzburger <asalzburger@gmail.com>

paulgessinger modified the milestones: next, v42.1.0 Jul 21, 2025

mimodekjaer pushed a commit to mimodekjaer/acts that referenced this pull request Aug 7, 2025

revert: feat: move ROOT surface material map I/O to plugins [originally

041cc38

acts-project#4400] (acts-project#4424) This reverts commit d036765.

njacazio pushed a commit to AliceO2Group/acts that referenced this pull request Nov 8, 2025

revert: feat: move ROOT surface material map I/O to plugins [originally

8e12177

acts-project#4400] (acts-project#4424) This reverts commit d036765.

Conversation

asalzburger commented Jul 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jul 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📊: Physics performance monitoring for c94bc8a

physmon summary

Uh oh!

paulgessinger left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

asalzburger commented Jul 12, 2025

Uh oh!

Uh oh!

sonarqubecloud bot commented Jul 12, 2025

Quality Gate passed

Uh oh!

paulgessinger commented Jul 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

asalzburger commented Jul 13, 2025

Uh oh!

asalzburger commented Jul 13, 2025

Uh oh!

paulgessinger commented Jul 14, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

asalzburger commented Jul 8, 2025 •

edited

Loading

github-actions bot commented Jul 8, 2025 •

edited

Loading

📊: Physics performance monitoring for `c94bc8a`

paulgessinger commented Jul 13, 2025 •

edited

Loading