Skip to content

Commit

Permalink
fix: DetrayMaterialConversion for Gen2 (acts-project#3748)
Browse files Browse the repository at this point in the history 
The Detray material conversion was broken due to missing locality of the index map.

It renames the cache object to simply `Cache` since it does more now, and through the namespace it gets clear that it is the conversion cache.
  • Loading branch information
@asalzburger @Rosie-Hasan
asalzburger authored and Rosie-Hasan committed Nov 13, 2024
1 parent 60bc827 commit e4d6d3b
Show file tree
Hide file tree
Showing 7 changed files with 147 additions and 120 deletions.
37 changes: 33 additions & 4 deletions Plugins/Detray/include/Acts/Plugins/Detray/DetrayConversionUtils.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@

namespace Acts {

namespace Experimental {
class DetectorVolume;
}

using DetrayHostDetector = detray::detector<detray::default_metadata>;

namespace DetrayConversionUtils {
Expand All @@ -28,12 +32,37 @@ namespace DetrayConversionUtils {
///
/// This object is used to synchronize link information between the
/// different converters (geometry, material, surface grids)
struct GeometryIdCache {
/// This is a multimap to pass volume local surface link information
/// The portal splitting requires a multimap implementation here
std::multimap<GeometryIdentifier, unsigned long> localSurfaceLinks;
struct Cache {
/// Explicit constructor with detector volumes
///
/// @param detectorVolumes the number of detector volumes
Cache(const std::vector<const Acts::Experimental::DetectorVolume*>& dVolumes)
: detectorVolumes(dVolumes) {}

/// The volumes of the detector for index lookup
std::vector<const Acts::Experimental::DetectorVolume*> detectorVolumes;
/// This is a map to pass on volume link information
std::map<GeometryIdentifier, unsigned long> volumeLinks;
/// This is a multimap to pass volume local surface link information
/// The portal splitting requires a multimap implementation here
///
/// These are volume local, hence indexed per volumes
std::map<std::size_t, std::multimap<GeometryIdentifier, unsigned long>>
localSurfaceLinks;

/// Find the position of the volume to point to
///
/// @param volume the volume to find
///
/// @note throws exception if volume is not found
std::size_t volumeIndex(
const Acts::Experimental::DetectorVolume* volume) const {
auto candidate = std::ranges::find(detectorVolumes, volume);
if (candidate != detectorVolumes.end()) {
return std::distance(detectorVolumes.begin(), candidate);
}
throw std::invalid_argument("Volume not found in the cache");
}
};

/// Convert the binning option
Expand Down
8 changes: 4 additions & 4 deletions Plugins/Detray/include/Acts/Plugins/Detray/DetrayConverter.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,15 +54,15 @@ class DetrayConverter {
const Experimental::Detector& detector,
vecmem::memory_resource& mr, const Options& options) {
// The building cache object
DetrayConversionUtils::GeometryIdCache geoIdCache;
DetrayConversionUtils::Cache cCache(detector.volumes());

typename detector_t::name_map names = {{0u, detector.name()}};

// build detector
detray::detector_builder<typename detector_t::metadata> detectorBuilder{};
// (1) geometry
detray::io::detector_payload detectorPayload =
DetrayGeometryConverter::convertDetector(geoIdCache, gctx, detector,
DetrayGeometryConverter::convertDetector(cCache, gctx, detector,
logger());
detray::io::geometry_reader::convert<detector_t>(detectorBuilder, names,
detectorPayload);
Expand All @@ -72,7 +72,7 @@ class DetrayConverter {
if (options.convertMaterial) {
detray::io::detector_homogeneous_material_payload materialSlabsPayload =
DetrayMaterialConverter::convertHomogeneousSurfaceMaterial(
geoIdCache, detector, logger());
cCache, detector, logger());
detray::io::homogeneous_material_reader::convert<detector_t>(
detectorBuilder, names, std::move(materialSlabsPayload));
}
Expand All @@ -85,7 +85,7 @@ class DetrayConverter {
detray::io::material_id>
materialGridsPayload =
DetrayMaterialConverter::convertGridSurfaceMaterial(
geoIdCache, detector, logger());
cCache, detector, logger());
detray::io::material_map_reader<std::integral_constant<
std::size_t, 2>>::convert<detector_t>(detectorBuilder, names,
std::move(
Expand Down
25 changes: 11 additions & 14 deletions Plugins/Detray/include/Acts/Plugins/Detray/DetrayGeometryConverter.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,49 +64,46 @@ detray::io::surface_payload convertSurface(const GeometryContext& gctx,

/// Conversion method for Portal object to detray::portal payloads
///
/// @param cCache [in, out] object
/// @param gctx the geometry context
/// @param portal the portal to be converted
/// @param ip the portal index
/// @param volume the volume to which the portal belongs
/// @param orientedSurfaces the oriented surfaces of the portal
/// @param detectorVolumes the detector volumes for the link lookup
///
/// @note due to portal splitting this can add up in N portals for one initial one
///
/// @brief convert the acts portal to detray surface payload and populate the payload
std::vector<detray::io::surface_payload> convertPortal(
const GeometryContext& gctx, const Experimental::Portal& portal,
std::size_t ip, const Experimental::DetectorVolume& volume,
const std::vector<OrientedSurface>& orientedSurfaces,
const std::vector<const Experimental::DetectorVolume*>& detectorVolumes);
DetrayConversionUtils::Cache& cCache, const GeometryContext& gctx,
const Experimental::Portal& portal, std::size_t ip,
const Experimental::DetectorVolume& volume,
const std::vector<OrientedSurface>& orientedSurfaces);

/// Conversion method for volume objects to detray::volume payloads
///
/// @param geoIdCache [in, out] object
/// @param cCache [in, out] object
/// @param gctx the geometry context
/// @param volume the volume to be converted
/// @param detectorVolumes the detector volumes for the link lookup
/// @param logger the logger object for screen output
///
/// @return the volume_payload for portals and volumes by @param volume acts object
detray::io::volume_payload convertVolume(
DetrayConversionUtils::GeometryIdCache& geoIdCache,
const GeometryContext& gctx, const Experimental::DetectorVolume& volume,
const std::vector<const Experimental::DetectorVolume*>& detectorVolumes,
const Acts::Logger& logger);
DetrayConversionUtils::Cache& cCache, const GeometryContext& gctx,
const Experimental::DetectorVolume& volume, const Acts::Logger& logger);

/// Conversion method for detector objects to detray::detector payload
///
/// @param geoIdCache [in, out] object
/// @param cCache [in, out] object
/// @param gctx the geometry context
/// @param detector the detector to be converted
/// @param logger the logger object for screen output
///
/// @return the detector_payload for portals and volumes by @param detector acts object
detray::io::detector_payload convertDetector(
DetrayConversionUtils::GeometryIdCache& geoIdCache,
const GeometryContext& gctx, const Experimental::Detector& detector,
const Acts::Logger& logger);
DetrayConversionUtils::Cache& cCache, const GeometryContext& gctx,
const Experimental::Detector& detector, const Acts::Logger& logger);

} // namespace DetrayGeometryConverter
} // namespace Acts
16 changes: 8 additions & 8 deletions Plugins/Detray/include/Acts/Plugins/Detray/DetrayMaterialConverter.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,15 @@ detray::io::material_slab_payload convertMaterialSlab(

/// Conversion method for homogeneous material
///
/// @param geoIdCache object to have the link association from the geometry building
/// @param cCache object to have the link association from the geometry building
/// @param detector the detector object
/// @param logger the logger object for screen output
///
/// @return the volume_payload for portals and volumes by @param volume acts object
detray::io::detector_homogeneous_material_payload
convertHomogeneousSurfaceMaterial(
const DetrayConversionUtils::GeometryIdCache& geoIdCache,
const Experimental::Detector& detector, const Logger& logger);
convertHomogeneousSurfaceMaterial(const DetrayConversionUtils::Cache& cCache,
const Experimental::Detector& detector,
const Logger& logger);

/// Conversion method for grid based surface material
///
Expand All @@ -58,16 +58,16 @@ convertGridSurfaceMaterial(const ISurfaceMaterial& material,

/// Conversion method for material grids
///
/// @param geoIdCache object to have the link association from the geometry building
/// @param cCache object to have the link association from the geometry building
/// @param detector the detector object
/// @param logger the logger object for screen output
///
/// @return the volume_payload for portals and volumes by @param volume acts object
detray::io::detector_grids_payload<detray::io::material_slab_payload,
detray::io::material_id>
convertGridSurfaceMaterial(
const DetrayConversionUtils::GeometryIdCache& geoIdCache,
const Experimental::Detector& detector, const Logger& logger);
convertGridSurfaceMaterial(const DetrayConversionUtils::Cache& cCache,
const Experimental::Detector& detector,
const Logger& logger);

} // namespace DetrayMaterialConverter

Expand Down
74 changes: 29 additions & 45 deletions Plugins/Detray/src/DetrayGeometryConverter.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,25 +27,6 @@

using namespace detray;

namespace {

/// Find the position of the volume to point to
///
/// @param volume the volume to find
/// @param the collection of volumes
///
/// @note return -1 if not found, to be interpreted by the caller
int findVolume(
const Acts::Experimental::DetectorVolume* volume,
const std::vector<const Acts::Experimental::DetectorVolume*>& volumes) {
auto candidate = std::ranges::find(volumes, volume);
if (candidate != volumes.end()) {
return std::distance(volumes.begin(), candidate);
}
return -1;
}
} // namespace

detray::io::transform_payload Acts::DetrayGeometryConverter::convertTransform(
const Transform3& t) {
detray::io::transform_payload tfPayload;
Expand Down Expand Up @@ -87,10 +68,10 @@ detray::io::surface_payload Acts::DetrayGeometryConverter::convertSurface(

std::vector<detray::io::surface_payload>
Acts::DetrayGeometryConverter::convertPortal(
const GeometryContext& gctx, const Experimental::Portal& portal,
std::size_t ip, const Experimental::DetectorVolume& volume,
const std::vector<Acts::OrientedSurface>& orientedSurfaces,
const std::vector<const Experimental::DetectorVolume*>& detectorVolumes) {
DetrayConversionUtils::Cache& cCache, const GeometryContext& gctx,
const Experimental::Portal& portal, std::size_t ip,
const Experimental::DetectorVolume& volume,
const std::vector<Acts::OrientedSurface>& orientedSurfaces) {
std::vector<detray::io::surface_payload> portals{};

const RegularSurface& surface = portal.surface();
Expand Down Expand Up @@ -135,7 +116,7 @@ Acts::DetrayGeometryConverter::convertPortal(
// in order to make sure the size is adjusted
if (singleLink != nullptr) {
// Single link can be written out
std::size_t vLink = findVolume(singleLink->object(), detectorVolumes);
std::size_t vLink = cCache.volumeIndex(singleLink->object());
auto portalPayload = convertSurface(gctx, *surfaceAdjusted, true);
portalPayload.mask.volume_link.link = vLink;
portals.push_back(portalPayload);
Expand All @@ -161,10 +142,10 @@ Acts::DetrayGeometryConverter::convertPortal(
auto surfaceType = surfaceAdjusted->type();
std::vector<unsigned int> vIndices = {};
for (const auto& v : volumes) {
vIndices.push_back(findVolume(v, detectorVolumes));
vIndices.push_back(cCache.volumeIndex(v));
}

// Pick the surface dimension - via poly
// Pick the surface dimension
std::array<ActsScalar, 2u> clipRange = {0., 0.};
std::vector<ActsScalar> boundValues = surfaceAdjusted->bounds().values();
if (surfaceType == Surface::SurfaceType::Cylinder &&
Expand Down Expand Up @@ -227,6 +208,7 @@ Acts::DetrayGeometryConverter::convertPortal(
0., 0.,
clippedBoundaries[ib - 1u] +
subBoundValues[CylinderBounds::BoundValues::eHalfLengthZ]));

auto subSurface =
Surface::makeShared<CylinderSurface>(subTransform, subBounds);
subSurface->assignGeometryId(surface.geometryId());
Expand All @@ -251,6 +233,7 @@ Acts::DetrayGeometryConverter::convertPortal(
auto subBounds = std::make_shared<RadialBounds>(subBoundValues);
auto subSurface = Surface::makeShared<DiscSurface>(
portal.surface().transform(gctx), subBounds);

subSurface->assignGeometryId(surface.geometryId());
auto portalPayload = convertSurface(gctx, *subSurface, true);
portalPayload.mask.volume_link.link = clippedIndices[ib - 1u];
Expand All @@ -260,7 +243,6 @@ Acts::DetrayGeometryConverter::convertPortal(
}

} else {
// End of world portal
// Write surface with invalid link
auto portalPayload = convertSurface(gctx, *surfaceAdjusted, true);
using NavigationLink =
Expand All @@ -271,35 +253,35 @@ Acts::DetrayGeometryConverter::convertPortal(
portals.push_back(portalPayload);
}
}

return portals;
}

detray::io::volume_payload Acts::DetrayGeometryConverter::convertVolume(
DetrayConversionUtils::GeometryIdCache& geoIdCache,
const GeometryContext& gctx,
DetrayConversionUtils::Cache& cCache, const GeometryContext& gctx,
const Acts::Experimental::DetectorVolume& volume,
const std::vector<const Experimental::DetectorVolume*>& detectorVolumes,
const Acts::Logger& logger) {
ACTS_DEBUG("DetrayGeometryConverter: converting volume "
<< volume.name() << " with " << volume.surfaces().size()
<< " surfaces and " << volume.portals().size() << " portals");

detray::io::volume_payload volumePayload;
std::size_t volumeIndex = cCache.volumeIndex(&volume);
volumePayload.name = volume.name();
volumePayload.index.link = findVolume(&volume, detectorVolumes);
volumePayload.index.link = volumeIndex;
volumePayload.transform = convertTransform(volume.transform(gctx));

// Remember the link
geoIdCache.volumeLinks[volume.geometryId()] = volumePayload.index.link;
cCache.volumeLinks[volume.geometryId()] = volumePayload.index.link;

std::multimap<GeometryIdentifier, unsigned long> localSurfaceLinks;

// iterate over surfaces and portals keeping the same surf_pd.index_in_coll
std::size_t sIndex = 0;
for (const auto surface : volume.surfaces()) {
io::surface_payload surfacePayload = convertSurface(gctx, *surface, false);
// Set the index in the collection & remember it in the cache
surfacePayload.index_in_coll = sIndex++;
geoIdCache.localSurfaceLinks.insert(
localSurfaceLinks.insert(
{surface->geometryId(), surfacePayload.index_in_coll.value()});
// Set mask to volume link
surfacePayload.mask.volume_link.link =
Expand All @@ -314,30 +296,32 @@ detray::io::volume_payload Acts::DetrayGeometryConverter::convertVolume(
int portalCounter = 0;
for (const auto& [ip, p] : enumerate(volume.portals())) {
auto portals =
convertPortal(gctx, *p, ip, volume, orientedSurfaces, detectorVolumes);

convertPortal(cCache, gctx, *p, ip, volume, orientedSurfaces);
ACTS_VERBOSE(" > portal " << ip << " split into " << portals.size()
<< " surfaces");
GeometryIdentifier geoID = p->surface().geometryId();
std::for_each(portals.begin(), portals.end(), [&](auto& portalPayload) {
// Set the index in the collection & remember it in the cache
portalPayload.index_in_coll = sIndex++;
geoIdCache.localSurfaceLinks.insert(
{geoID, portalPayload.index_in_coll.value()});
localSurfaceLinks.insert({geoID, portalPayload.index_in_coll.value()});
// Add it to the surfaces
volumePayload.surfaces.push_back(portalPayload);
portalCounter++;
});
}
ACTS_VERBOSE(" > " << volume.portals().size()
<< " initial ACTS portals split into final "
<< portalCounter << " detray portals");
cCache.localSurfaceLinks[volumeIndex] = localSurfaceLinks;
ACTS_DEBUG(" > " << volume.portals().size()
<< " initial ACTS portals split into final " << portalCounter
<< " detray portals");
ACTS_VERBOSE(" > Local surface link cache has " << localSurfaceLinks.size()
<< " entries");

return volumePayload;
}

detray::io::detector_payload Acts::DetrayGeometryConverter::convertDetector(
DetrayConversionUtils::GeometryIdCache& geoIdCache,
const GeometryContext& gctx, const Acts::Experimental::Detector& detector,
const Acts::Logger& logger) {
DetrayConversionUtils::Cache& cCache, const GeometryContext& gctx,
const Acts::Experimental::Detector& detector, const Acts::Logger& logger) {
ACTS_DEBUG("DetrayGeometryConverter: converting detector"
<< detector.name() << " with " << detector.volumes().size()
<< " volumes.");
Expand All @@ -347,7 +331,7 @@ detray::io::detector_payload Acts::DetrayGeometryConverter::convertDetector(

for (const auto volume : detector.volumes()) {
detectorPayload.volumes.push_back(
convertVolume(geoIdCache, gctx, *volume, detector.volumes(), logger));
convertVolume(cCache, gctx, *volume, logger));
}

return detectorPayload;
Expand Down
Loading

0 comments on commit e4d6d3b

Please sign in to comment.