Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Introduce the usage of the geomodel converters for the Muon System #3348

Closed
wants to merge 42 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
654904a
geomodel conversion of the mdt tubes for the muon system
dimitra97 Jul 3, 2024
e57bb00
add database to publish it to people
dimitra97 Jul 3, 2024
cf77083
format python
dimitra97 Jul 3, 2024
5a871ab
spelling fix
dimitra97 Jul 3, 2024
a9386d6
Update Plugins/GeoModel/src/GeoModelDetectorSurfaceFactory.cpp
dimitra97 Jul 3, 2024
c7dccce
Update Plugins/GeoModel/src/GeoModelDetectorSurfaceFactory.cpp
dimitra97 Jul 3, 2024
9cb2c8f
pr comments
dimitra97 Jul 3, 2024
9581e28
Merge branch 'geomodel-muons' of github.com:dimitra97/acts into geomo…
dimitra97 Jul 3, 2024
08419cd
format check
dimitra97 Jul 3, 2024
f749c3b
fix
dimitra97 Jul 3, 2024
4cc0263
Update Plugins/GeoModel/include/Acts/Plugins/GeoModel/GeoModelConvert…
dimitra97 Jul 3, 2024
65320d4
Update Plugins/GeoModel/src/GeoModelDetectorSurfaceFactory.cpp
dimitra97 Jul 3, 2024
3669c82
forgotten comments
dimitra97 Jul 3, 2024
4e3b05b
Merge branch 'geomodel-muons' of github.com:dimitra97/acts into geomo…
dimitra97 Jul 3, 2024
8dc4cb2
Update Plugins/GeoModel/include/Acts/Plugins/GeoModel/GeoModelDetecto…
dimitra97 Jul 3, 2024
d6aa993
again forgotten comments that andreas sawgit add Examples/Scripts/Pyt…
dimitra97 Jul 3, 2024
baf84cf
Merge branch 'geomodel-muons' of github.com:dimitra97/acts into geomo…
dimitra97 Jul 3, 2024
e8965ff
format check
dimitra97 Jul 3, 2024
8976721
remove the database
dimitra97 Jul 3, 2024
fc7bd0f
remove include not needed
dimitra97 Jul 3, 2024
aca782f
remove forgotten comment again!
dimitra97 Jul 3, 2024
877a6d1
Update Plugins/GeoModel/src/GeoModelDetectorSurfaceFactory.cpp
dimitra97 Jul 4, 2024
9563956
Update Plugins/GeoModel/src/GeoModelDetectorSurfaceFactory.cpp
dimitra97 Jul 4, 2024
bb1a21b
changes after using the 6.2 geomodel version and the new database
dimitra97 Jul 4, 2024
ca05ce0
Merge branch 'geomodel-muons' of github.com:dimitra97/acts into geomo…
dimitra97 Jul 4, 2024
43e3b92
Merge branch 'acts-project:main' into geomodel-muons
dimitra97 Jul 4, 2024
8a8eb1f
Merge branch 'main' into geomodel-muons
dimitra97 Jul 4, 2024
8df082d
remove some forgotten couts
dimitra97 Jul 5, 2024
ff65f65
Merge branch 'geomodel-muons' of github.com:dimitra97/acts into geomo…
dimitra97 Jul 5, 2024
c32c40b
Merge branch 'main' into geomodel-muons
dimitra97 Jul 5, 2024
1154122
Merge branch 'main' into geomodel-muons
dimitra97 Jul 9, 2024
f021360
Changes to fix finding geomodel on system and first round of converter
Jun 3, 2024
6f0b21b
format changes
Jun 7, 2024
5279972
python format --i hope
dimitra97 Jul 9, 2024
0b4ebd0
Delete Plugins/GeoModel/include/Acts/Plugins/GeoModel/GeoModelToDetVo…
dimitra97 Jul 9, 2024
1b75772
Delete Plugins/GeoModel/src/GeoModelToDetVol.cpp
dimitra97 Jul 9, 2024
8f114cc
geomodel version to 6.2
dimitra97 Jul 9, 2024
5bcc1d4
Merge branch 'geomodel-muons' of github.com:dimitra97/acts into geomo…
dimitra97 Jul 9, 2024
1c1daef
pass volume by reference
dimitra97 Jul 9, 2024
eaffacd
Merge branch 'main' into geomodel-muons
dimitra97 Jul 9, 2024
daccdac
Merge branch 'main' into geomodel-muons
dimitra97 Jul 18, 2024
9ba3f7d
Merge branch 'main' into geomodel-muons
dimitra97 Jul 24, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Examples/Python/src/GeoModel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,8 @@ void addGeoModel(Context& ctx) {
py::class_<Acts::GeoModelDetectorSurfaceFactory::Config>(f, "Config")
.def(py::init<>())
.def_readwrite(
"shapeConverters",
&Acts::GeoModelDetectorSurfaceFactory::Config::shapeConverters)
"convertSubVolumes",
&Acts::GeoModelDetectorSurfaceFactory::Config::convertSubVolumes)
.def_readwrite("nameList",
&Acts::GeoModelDetectorSurfaceFactory::Config::nameList)
.def_readwrite(
Expand Down
Binary file added Examples/Scripts/Python/ATLAS-R3-MUONTEST_v2.db
dimitra97 marked this conversation as resolved.
Show resolved Hide resolved
Binary file not shown.
173 changes: 106 additions & 67 deletions Examples/Scripts/Python/geomodel.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,26 @@ def main():
help="List of Queries for Published full phys volumes",
)

p.add_argument(
"-n",
"--name-list",
type=str,
nargs="+",
default=[],
help="List of Name List for the Surface Factory",

)

p.add_argument(
"-ml",
"--material-list",
type=str,
nargs="+",
default=[],
help="List of Material List for the Surface Factory",

)

p.add_argument(
"--table-name",
type=str,
Expand Down Expand Up @@ -56,6 +76,13 @@ def main():
"-m", "--map", type=str, default="", help="Input file for the material map"
)

p.add_argument(
"--convert-subvols",
help="Convert the children of the top level full phys vol",
action="store_true",
default=False,
)

p.add_argument(
"--output-svg",
help="Write the surfaces to SVG files",
Expand Down Expand Up @@ -84,6 +111,13 @@ def main():
default=False,
)

p.add_argument(
"--enable-blueprint",
help="Enable the usage of the blueprint",
action="store_true",
default=False,
)

args = p.parse_args()

gContext = acts.GeometryContext()
Expand All @@ -98,11 +132,14 @@ def main():
gmTree = acts.geomodel.readFromDb(args.input)

gmFactoryConfig = gm.GeoModelDetectorSurfaceFactory.Config()
gmFactoryConfig.shapeConverters = [
gm.GeoBoxConverter(),
gm.GeoTrdConverter(),
gm.GeoIntersectionAnnulusConverter(),
]
# gmFactoryConfig.shapeConverters = [
# gm.GeoBoxConverter(),
# gm.GeoTrdConverter(),
# gm.GeoIntersectionAnnulusConverter(),
# ]
dimitra97 marked this conversation as resolved.
Show resolved Hide resolved
gmFactoryConfig.materialList = args.material_list
gmFactoryConfig.nameList = args.name_list
gmFactoryConfig.convertSubVolumes = args.convert_subvols
Comment on lines +133 to +135
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Validate the inputs, we must.

Safe handling of command line arguments, crucial it is. Add validation before assignment, you should.

+    # Validate material and name lists
+    def validate_list(lst, list_name):
+        if not all(isinstance(item, str) for item in lst):
+            raise ValueError(f"All {list_name} must be strings")
+
+    validate_list(args.material_list, "materials")
+    validate_list(args.name_list, "names")
     gmFactoryConfig.materialList = args.material_list
     gmFactoryConfig.nameList = args.name_list
     gmFactoryConfig.convertSubVolumes = args.convert_subvols
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
gmFactoryConfig.materialList = args.material_list
gmFactoryConfig.nameList = args.name_list
gmFactoryConfig.convertSubVolumes = args.convert_subvols
# Validate material and name lists
def validate_list(lst, list_name):
if not all(isinstance(item, str) for item in lst):
raise ValueError(f"All {list_name} must be strings")
validate_list(args.material_list, "materials")
validate_list(args.name_list, "names")
gmFactoryConfig.materialList = args.material_list
gmFactoryConfig.nameList = args.name_list
gmFactoryConfig.convertSubVolumes = args.convert_subvols

gmFactory = gm.GeoModelDetectorSurfaceFactory(gmFactoryConfig, logLevel)
# The options
gmFactoryOptions = gm.GeoModelDetectorSurfaceFactory.Options()
Expand All @@ -115,77 +152,79 @@ def main():
gmSurfaces = [ss[1] for ss in gmFactoryCache.sensitiveSurfaces]

# Construct the building hierarchy
gmBlueprintConfig = gm.GeoModelBlueprintCreater.Config()
gmBlueprintConfig.detectorSurfaces = gmSurfaces
gmBlueprintConfig.kdtBinning = [acts.Binning.z, acts.Binning.r]
#if the blueprint is enabled
if args.enable_blueprint:
gmBlueprintConfig = gm.GeoModelBlueprintCreater.Config()
gmBlueprintConfig.detectorSurfaces = gmSurfaces
gmBlueprintConfig.kdtBinning = [acts.Binning.z, acts.Binning.r]

gmBlueprintOptions = gm.GeoModelBlueprintCreater.Options()
gmBlueprintOptions.table = args.table_name
gmBlueprintOptions.topEntry = args.top_node
if len(args.top_node_bounds) > 0:
gmBlueprintOptions.topBoundsOverride = args.top_node_bounds
gmBlueprintOptions = gm.GeoModelBlueprintCreater.Options()
gmBlueprintOptions.table = args.table_name
gmBlueprintOptions.topEntry = args.top_node
if len(args.top_node_bounds) > 0:
gmBlueprintOptions.topBoundsOverride = args.top_node_bounds

gmBlueprintCreater = gm.GeoModelBlueprintCreater(gmBlueprintConfig, logLevel)
gmBlueprint = gmBlueprintCreater.create(gContext, gmTree, gmBlueprintOptions)
gmBlueprintCreater = gm.GeoModelBlueprintCreater(gmBlueprintConfig, logLevel)
gmBlueprint = gmBlueprintCreater.create(gContext, gmTree, gmBlueprintOptions)

gmCylindricalBuilder = gmBlueprint.convertToBuilder(logLevel)
gmCylindricalBuilder = gmBlueprint.convertToBuilder(logLevel)

# Top level geo id generator
gmGeoIdConfig = GeometryIdGenerator.Config()
gmGeoIdGenerator = GeometryIdGenerator(
gmGeoIdConfig, "GeoModelGeoIdGenerator", logLevel
)
gmGeoIdConfig = GeometryIdGenerator.Config()
gmGeoIdGenerator = GeometryIdGenerator(
gmGeoIdConfig, "GeoModelGeoIdGenerator", logLevel
)

# Create the detector builder
gmDetectorConfig = DetectorBuilder.Config()
gmDetectorConfig.name = args.top_node + "_DetectorBuilder"
gmDetectorConfig.builder = gmCylindricalBuilder
gmDetectorConfig.geoIdGenerator = gmGeoIdGenerator
gmDetectorConfig.materialDecorator = materialDecorator
gmDetectorConfig.auxiliary = (
# Create the detector builder
gmDetectorConfig = DetectorBuilder.Config()
gmDetectorConfig.name = args.top_node + "_DetectorBuilder"
gmDetectorConfig.builder = gmCylindricalBuilder
gmDetectorConfig.geoIdGenerator = gmGeoIdGenerator
gmDetectorConfig.materialDecorator = materialDecorator
gmDetectorConfig.auxiliary = (
"GeoModel based Acts::Detector from '" + args.input + "'"
)

gmDetectorBuilder = DetectorBuilder(gmDetectorConfig, args.top_node, logLevel)
detector = gmDetectorBuilder.construct(gContext)

materialSurfaces = detector.extractMaterialSurfaces()
print("Found ", len(materialSurfaces), " material surfaces")

# Output the detector to SVG
if args.output_svg:
surfaceStyle = acts.svg.Style()
surfaceStyle.fillColor = [5, 150, 245]
surfaceStyle.fillOpacity = 0.5

surfaceOptions = acts.svg.SurfaceOptions()
surfaceOptions.style = surfaceStyle

viewRange = acts.Extent([])
volumeOptions = acts.svg.DetectorVolumeOptions()
volumeOptions.surfaceOptions = surfaceOptions

xyRange = acts.Extent([[acts.Binning.z, [-50, 50]]])
zrRange = acts.Extent([[acts.Binning.phi, [-0.8, 0.8]]])

acts.svg.viewDetector(
gContext,
detector,
args.top_node,
[[ivol, volumeOptions] for ivol in range(detector.numberVolumes())],
[
["xy", ["sensitives", "portals"], xyRange],
["zr", ["", "", "materials"], zrRange],
],
args.output + "_detector",
)

# Output the internal navigation to SVG
if args.output_internals_svg:
for vol in detector.volumes():
acts.svg.viewInternalNavigation(
gContext, vol, [66, 111, 245, 245, 203, 66, 0.8], "/;:"
)
gmDetectorBuilder = DetectorBuilder(gmDetectorConfig, args.top_node, logLevel)
detector = gmDetectorBuilder.construct(gContext)

materialSurfaces = detector.extractMaterialSurfaces()
print("Found ", len(materialSurfaces), " material surfaces")

# Output the detector to SVG
if args.output_svg:
surfaceStyle = acts.svg.Style()
surfaceStyle.fillColor = [5, 150, 245]
surfaceStyle.fillOpacity = 0.5

surfaceOptions = acts.svg.SurfaceOptions()
surfaceOptions.style = surfaceStyle

viewRange = acts.Extent([])
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Remove the unused variable, we must.

Unused, this variable is. Remove it, we should.

-            viewRange = acts.Extent([])
🧰 Tools
🪛 Ruff (0.8.0)

196-196: Local variable viewRange is assigned to but never used

Remove assignment to unused variable viewRange

(F841)

volumeOptions = acts.svg.DetectorVolumeOptions()
volumeOptions.surfaceOptions = surfaceOptions

xyRange = acts.Extent([[acts.Binning.z, [-50, 50]]])
zrRange = acts.Extent([[acts.Binning.phi, [-0.8, 0.8]]])

acts.svg.viewDetector(
gContext,
detector,
args.top_node,
[[ivol, volumeOptions] for ivol in range(detector.numberVolumes())],
[
["xy", ["sensitives", "portals"], xyRange],
["zr", ["", "", "materials"], zrRange],
],
args.output + "_detector",
)

# Output the internal navigation to SVG
if args.output_internals_svg:
for vol in detector.volumes():
acts.svg.viewInternalNavigation(
gContext, vol, [66, 111, 245, 245, 203, 66, 0.8], "/;:"
)

# Output the surface to an OBJ file
if args.output_obj:
Expand Down
2 changes: 1 addition & 1 deletion Plugins/GeoModel/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ target_include_directories(
$<INSTALL_INTERFACE:${CMAKE_INSTALL_INCLUDEDIR}>)
target_link_libraries(
ActsPluginGeoModel
PUBLIC ActsCore GeoModelKernel GeoModelIO::GeoModelDBManager GeoModelIO::GeoModelRead)
PUBLIC ActsCore GeoModelKernel GeoModelCore::GeoModelHelpers GeoModelIO::GeoModelDBManager GeoModelIO::GeoModelRead)

install(
TARGETS ActsPluginGeoModel
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "Acts/Plugins/GeoModel/detail/GeoUnionDoubleTrdConverter.hpp"
#include "Acts/Utilities/Result.hpp"

#include <map>
dimitra97 marked this conversation as resolved.
Show resolved Hide resolved
#include <memory>
#include <tuple>

Expand Down Expand Up @@ -68,4 +69,24 @@ using GeoUnionDoubleTrdConverter =
detail::GenericGeoShapeConverter<GeoShapeUnion,
detail::GeoUnionDoubleTrdConverter>;

/// @brief The map that maps the converters with the shapes

std::shared_ptr<const IGeoShapeConverter> GeoShapesConverters(int geoShapeId) {
using ConverterLookUpMap =
std::map<int, std::shared_ptr<const IGeoShapeConverter>>;
dimitra97 marked this conversation as resolved.
Show resolved Hide resolved

static const ConverterLookUpMap converters{
{GeoBox::getClassTypeID(), std::make_shared<GeoBoxConverter>()},
{GeoShapeIntersection::getClassTypeID(),
std::make_shared<GeoIntersectionAnnulusConverter>()},
{GeoShapeShift::getClassTypeID(), std::make_shared<GeoShiftConverter>()},
{GeoTrd::getClassTypeID(), std::make_shared<GeoTrdConverter>()},
{GeoTube::getClassTypeID(), std::make_shared<GeoTubeConverter>()},
{GeoShapeUnion::getClassTypeID(),
std::make_shared<GeoUnionDoubleTrdConverter>()}};
ConverterLookUpMap::const_iterator itr = converters.find(geoShapeId);

return itr != converters.end() ? itr->second : nullptr;
};

} // namespace Acts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@

#include <memory>

#include <GeoModelKernel/GeoFullPhysVol.h>

class GeoFullPhysVol;
andiwand marked this conversation as resolved.
Show resolved Hide resolved
class GeoVPhysVol;

namespace Acts {

Expand Down Expand Up @@ -48,9 +51,8 @@ class GeoModelDetectorElement : public DetectorElementBase {
/// @return a shared pointer to an instance of the detector element
template <typename SurfaceType, typename BoundsType>
static std::shared_ptr<GeoModelDetectorElement> createDetectorElement(
const GeoFullPhysVol& geoPhysVol,
const std::shared_ptr<BoundsType> bounds, const Transform3& sfTransform,
ActsScalar thickness) {
PVConstLink geoPhysVol, const std::shared_ptr<BoundsType> bounds,
andiwand marked this conversation as resolved.
Show resolved Hide resolved
const Transform3& sfTransform, ActsScalar thickness) {
// First create the detector element with a nullptr
auto detElement = std::make_shared<GeoModelDetectorElement>(
geoPhysVol, nullptr, sfTransform, thickness);
Expand All @@ -65,7 +67,7 @@ class GeoModelDetectorElement : public DetectorElementBase {
/// @param surface the representing surface
/// @param sfTransform the surface transform
/// @param thickness the thickness of the detector element
GeoModelDetectorElement(const GeoFullPhysVol& geoPhysVol,
GeoModelDetectorElement(PVConstLink geoPhysVol,
std::shared_ptr<Surface> surface,
const Transform3& sfTransform, ActsScalar thickness);

Expand All @@ -84,7 +86,7 @@ class GeoModelDetectorElement : public DetectorElementBase {
ActsScalar thickness() const override;

/// @return to the Geant4 physical volume
const GeoFullPhysVol& physicalVolume() const;
PVConstLink physicalVolume() const;

private:
/// Attach a surface
Expand All @@ -95,7 +97,7 @@ class GeoModelDetectorElement : public DetectorElementBase {
}

/// The GeoModel full physical volume
const GeoFullPhysVol* m_geoPhysVol{nullptr};
PVConstLink m_geoPhysVol{nullptr};
/// The surface
std::shared_ptr<Surface> m_surface;
/// The global transformation before the volume
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
#include <tuple>
#include <vector>

#include <GeoModelHelpers/getChildNodesWithTrf.h>

class GeoShape;
class GeoFullPhysVol;

Expand All @@ -38,12 +40,16 @@ class GeoModelDetectorSurfaceFactory {

// Configuration struct for the Detector surface factory
struct Config {
/// The shape converters to be use
std::vector<std::shared_ptr<const IGeoShapeConverter>> shapeConverters = {};
/// List for names to match
// /// The shape converters to be use
// std::vector<std::shared_ptr<const IGeoShapeConverter>> shapeConverters =
// {};
// /// List for names to match
dimitra97 marked this conversation as resolved.
Show resolved Hide resolved
std::vector<std::string> nameList;
/// List for materials to match
// /// List for materials to match
dimitra97 marked this conversation as resolved.
Show resolved Hide resolved
std::vector<std::string> materialList;

/// boolean flag to build subvolumes
bool convertSubVolumes = false;
};

/// Nested cache that records the conversion status
Expand Down Expand Up @@ -88,6 +94,24 @@ class GeoModelDetectorSurfaceFactory {

/// Private access to the logger
const Logger& logger() const { return *m_logger; }

/// Private helper method that does the conversion to surfaces
///
/// @param geoPV the pointer to the physical volume
/// @param transform the transform to the global frame
/// @param sensitives the vector which we fill with the converted surfaces
void convertSensitive(
PVConstLink geoPV, const Acts::Transform3& transform,
std::vector<Acts::GeoModelSensitiveSurface>& sensitives);

/// Private helper method that finds the subvolumes from the top-level full
/// physical volume
///
/// @param geoPV the pointer to the physical volume
/// @param matchFunc the function that checks if the volume matches the name and material requirements
std::vector<GeoChildNodeWithTrf> findAllSubVolumes(
PVConstLink geoPV,
std::function<bool(std::string, PVConstLink)> matchFunc);
};

} // namespace Acts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ namespace Acts {

struct GeoModelTree {
std::shared_ptr<GeoModelIO::ReadGeoModel> geoReader = nullptr;
GeoVPhysVol* worldVolume = nullptr;
PVConstLink worldVolume{nullptr};
std::string worldVolumeName = "World";
};
} // namespace Acts
Loading
Loading