Skip to content

Commit

Permalink
chore: Sonar fixes to detray and geomodel plugins
Browse files Browse the repository at this point in the history
  • Loading branch information
paulgessinger committed Aug 15, 2024
1 parent 54eeb65 commit fe328ae
Show file tree
Hide file tree
Showing 6 changed files with 37 additions and 40 deletions.
2 changes: 1 addition & 1 deletion Examples/Python/src/GeoModel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ void addGeoModel(Context& ctx) {
std::shared_ptr<Acts::GeoModelBlueprintCreater::Blueprint>>(
gm, "Blueprint")
.def("convertToBuilder",
[](Acts::GeoModelBlueprintCreater::Blueprint& self,
[](const Acts::GeoModelBlueprintCreater::Blueprint& self,
Acts::Logging::Level level) {
// It's a container builder
return std::make_shared<
Expand Down
38 changes: 21 additions & 17 deletions Plugins/Detray/src/DetrayConverter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@

#include "detray/io/frontend/detector_writer.hpp"

using namespace detray;

namespace {

/// Find the position of the volume to point to
Expand All @@ -36,9 +34,9 @@ namespace {
int findVolume(
const Acts::Experimental::DetectorVolume* volume,
const std::vector<const Acts::Experimental::DetectorVolume*>& volumes) {
auto candidate = std::find(volumes.begin(), volumes.end(), volume);
if (candidate != volumes.end()) {
return std::distance(volumes.begin(), candidate);
if (auto candidate = std::ranges::find(volumes, volume);
candidate != volumes.end()) {
return static_cast<int>(std::distance(volumes.begin(), candidate));
}
return -1;
}
Expand Down Expand Up @@ -70,25 +68,30 @@ detray::io::mask_payload Acts::DetrayConverter::convertMask(
const Acts::SurfaceBounds& bounds, bool portal) {
detray::io::mask_payload maskPayload;
auto [shape, boundaries] = DetrayJsonHelper::maskFromBounds(bounds, portal);
maskPayload.shape = static_cast<io::mask_payload::mask_shape>(shape);
maskPayload.boundaries = static_cast<std::vector<real_io>>(boundaries);
maskPayload.shape = static_cast<detray::io::mask_payload::mask_shape>(shape);
maskPayload.boundaries =
static_cast<std::vector<detray::real_io>>(boundaries);
// default maskPayload.volume_link

return maskPayload;
}

detray::io::surface_payload Acts::DetrayConverter::convertSurface(
const Acts::GeometryContext& gctx, const Surface& surface, bool portal) {
using enum detray::surface_id;

detray::io::surface_payload surfacePayload;

surfacePayload.transform = convertTransform(surface.transform(gctx));
surfacePayload.source = surface.geometryId().value();
surfacePayload.barcode = std::nullopt;
surfacePayload.type = static_cast<detray::surface_id>(
portal ? surface_id::e_portal
: (surface.geometryId().sensitive() > 0
? detray::surface_id::e_sensitive
: detray::surface_id::e_passive));

if (portal) {
surfacePayload.type = e_portal;
} else {
surfacePayload.type =
(surface.geometryId().sensitive() > 0 ? e_sensitive : e_passive);
}
surfacePayload.mask = convertMask(surface.bounds());
return surfacePayload;
}
Expand Down Expand Up @@ -134,7 +137,7 @@ std::vector<detray::io::surface_payload> Acts::DetrayConverter::convertPortal(
dynamic_cast<const Acts::Experimental::SingleDetectorVolumeNavigation*>(
instance);

auto [surfaceAdjusted, insidePointer] = orientedSurfaces[ip];
const auto& [surfaceAdjusted, insidePointer] = orientedSurfaces[ip];

// Single link detected - just write it out, we use the oriented surface
// in order to make sure the size is adjusted
Expand All @@ -159,8 +162,8 @@ std::vector<detray::io::surface_payload> Acts::DetrayConverter::convertPortal(

// Apply the correction from local to global boundaries
ActsScalar gCorr = VectorHelpers::cast(transform.translation(), cast);
std::for_each(boundaries.begin(), boundaries.end(),
[&gCorr](ActsScalar& b) { b -= gCorr; });
std::ranges::for_each(boundaries,
[&gCorr](ActsScalar& b) { b -= gCorr; });

// Get the volume indices
auto surfaceType = surfaceAdjusted->type();
Expand Down Expand Up @@ -290,7 +293,8 @@ detray::io::volume_payload Acts::DetrayConverter::convertVolume(
// 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);
detray::io::surface_payload surfacePayload =
convertSurface(gctx, *surface, false);

surfacePayload.index_in_coll = sIndex++;
surfacePayload.mask.volume_link.link =
Expand All @@ -305,7 +309,7 @@ detray::io::volume_payload Acts::DetrayConverter::convertVolume(
for (const auto& [ip, p] : enumerate(volume.portals())) {
auto portals =
convertPortal(gctx, *p, ip, volume, orientedSurfaces, detectorVolumes);
std::for_each(portals.begin(), portals.end(), [&](auto& portalPayload) {
std::ranges::for_each(portals, [&](auto& portalPayload) {
portalPayload.index_in_coll = sIndex++;
volumePayload.surfaces.push_back(portalPayload);
portalCounter++;
Expand Down
12 changes: 6 additions & 6 deletions Plugins/GeoModel/src/GeoModelDetectorSurfaceFactory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,17 +60,17 @@ void Acts::GeoModelDetectorSurfaceFactory::construct(
return true;
}

match |= std::any_of(
m_cfg.nameList.begin(), m_cfg.nameList.end(),
[&](const auto &n) { return name.find(n) != std::string::npos; });
match |= std::ranges::any_of(m_cfg.nameList, [&](const auto &n) {
return name.find(n) != std::string::npos;
});
if (match) {
return true;
}

const auto &matStr = fpv.getLogVol()->getMaterial()->getName();
match |= std::any_of(
m_cfg.materialList.begin(), m_cfg.materialList.end(),
[&](const auto &m) { return matStr.find(m) != std::string::npos; });
match |= std::ranges::any_of(m_cfg.materialList, [&](const auto &m) {
return matStr.find(m) != std::string::npos;
});

return match;
};
Expand Down
1 change: 0 additions & 1 deletion Plugins/GeoModel/src/detail/GeoShiftConverter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ Result<GeoModelSensitiveSurface> impl(const GeoFullPhysVol& geoFPV,

if (trd == nullptr) {
return GeoModelConversionError::WrongShapeForConverter;
;
}

const Transform3& shift = geoShift.getX();
Expand Down
5 changes: 2 additions & 3 deletions Plugins/GeoModel/src/detail/GeoUnionDoubleTrdConverter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,8 @@ bool trapezoidsAreMergeable(const std::vector<Acts::Vector3> &vtxsa,
auto dist1 = distanceLinePoint(vtxsa[3], vtxsb[0], P1);

auto P2 = vtxsa[1] + 0.5 * (vtxsb[2] - vtxsa[1]);
auto dist2 = distanceLinePoint(vtxsa[2], vtxsb[1], P2);

if (dist1 > 1.e-3 || dist2 > 1.e-3) {
if (auto dist2 = distanceLinePoint(vtxsa[2], vtxsb[1], P2);
dist1 > 1.e-3 || dist2 > 1.e-3) {
return false;
}

Expand Down
19 changes: 7 additions & 12 deletions Plugins/Json/src/DetrayJsonHelper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,28 +58,23 @@ void addVolumeLink(nlohmann::json& jSurface, int vLink) {
}

std::size_t accelerationLink(std::span<const BinningValue> casts) {
using enum BinningValue;
// Default is `brute_force`
std::size_t accLink = 0u;
if (casts.size() == 2u) {
if (casts[0u] == BinningValue::binX && casts[1u] == BinningValue::binY) {
if (casts[0u] == binX && casts[1u] == binY) {
accLink = 1u;
} else if (casts[0u] == BinningValue::binR &&
casts[1u] == BinningValue::binPhi) {
} else if (casts[0u] == binR && casts[1u] == binPhi) {
accLink = 3u;
} else if (casts[0u] == BinningValue::binZ &&
casts[1u] == BinningValue::binPhi) {
} else if (casts[0u] == binZ && casts[1u] == binPhi) {
accLink = 4u;
} else if (casts[0u] == BinningValue::binZ &&
casts[1u] == BinningValue::binR) {
} else if (casts[0u] == binZ && casts[1u] == binR) {
accLink = 5u;
}
} else if (casts.size() == 3u) {
if (casts[0u] == BinningValue::binX && casts[1u] == BinningValue::binY &&
casts[2u] == BinningValue::binZ) {
if (casts[0u] == binX && casts[1u] == binY && casts[2u] == binZ) {
accLink = 2u;
} else if (casts[0u] == BinningValue::binZ &&
casts[1u] == BinningValue::binPhi &&
casts[2u] == BinningValue::binR) {
} else if (casts[0u] == binZ && casts[1u] == binPhi && casts[2u] == binR) {
accLink = 5u;
}
}
Expand Down

0 comments on commit fe328ae

Please sign in to comment.