From f6aa8bbb8023f05f00b649af6edf8fdbe7a1969b Mon Sep 17 00:00:00 2001
From: Johannes Josef Junggeburth <johannes.josef.junggeburth@cern.ch>
Date: Thu, 30 Jan 2025 17:23:02 +0100
Subject: [PATCH 1/5] Turn Query into a std::optional & add unit test
---
.../GeoModelKernel/GeoModelKernel/Query.h | 51 ++++--------
.../GeoModelKernel/tests/testQuery.cxx | 78 +++++++++++++++++++
2 files changed, 92 insertions(+), 37 deletions(-)
create mode 100644 GeoModelCore/GeoModelKernel/tests/testQuery.cxx
diff --git a/GeoModelCore/GeoModelKernel/GeoModelKernel/Query.h b/GeoModelCore/GeoModelKernel/GeoModelKernel/Query.h
index f3cde970c..60dfaf7f3 100644
--- a/GeoModelCore/GeoModelKernel/GeoModelKernel/Query.h
+++ b/GeoModelCore/GeoModelKernel/GeoModelKernel/Query.h
@@ -1,5 +1,5 @@
/*
- Copyright (C) 2002-2024 CERN for the benefit of the ATLAS collaboration
+ Copyright (C) 2002-2025 CERN for the benefit of the ATLAS collaboration
*/
#ifndef GEOMODELKERNEL_QUERY_H
@@ -18,54 +18,31 @@
* based on Barton & Nackman's "Scientific and Engineering C++"
*/
-#include <stdexcept>
-template < class T > class Query
+#include <GeoModelKernel/throwExcept.h>
+#include <optional>
+template < class T > class Query: public std::optional<T>
{
public:
-
- // Constructor:
- inline Query (const T &);
-
- // Default constructor:
- inline Query ();
+ /// Use all constructors from the std::optional
+ using std::optional<T>::optional;
// Convert to "T"
- inline operator T () const;
+ inline operator T () const;
// Test Validity
inline bool isValid () const;
-private:
-
- bool m_failed;
- T m_instance;
-
};
-
-template < class T > inline Query < T >::Query (const T & t):
-m_failed (false),
-m_instance (t)
-{
-}
-
-template < class T > inline Query < T >::Query ():
-m_failed (true),
-m_instance (T())
-{
+template < class T > inline Query < T >::operator T () const {
+ if (!this->isValid()){
+ THROW_EXCEPTION("Nothing has been saved in query of type "<<typeid(T).name());
+ }
+ return this->value_or(T{});
}
-
-template < class T > inline Query < T >::operator T () const
-{
- if (m_failed)
- throw std::range_error ("Failed query");
- return m_instance;
-}
-
-template < class T > inline bool Query < T >::isValid () const
-{
- return !m_failed;
+template < class T > inline bool Query < T >::isValid () const {
+ return this->has_value();
}
diff --git a/GeoModelCore/GeoModelKernel/tests/testQuery.cxx b/GeoModelCore/GeoModelKernel/tests/testQuery.cxx
new file mode 100644
index 000000000..61cc302d8
--- /dev/null
+++ b/GeoModelCore/GeoModelKernel/tests/testQuery.cxx
@@ -0,0 +1,78 @@
+/*
+ Copyright (C) 2002-2025 CERN for the benefit of the ATLAS collaboration
+ */
+#include "GeoModelKernel/Query.h"
+#include "GeoModelKernel/throwExcept.h"
+
+#include <stdlib.h>
+#include <iostream>
+
+
+#define RETURN_FAILURE(MSG) { \
+ std::cerr<<__FILE__<<":"<<__LINE__<<" -- "<<MSG<<std::endl; \
+ return EXIT_FAILURE; \
+}
+int main() {
+
+ {
+ /// Test that the default constructor generates an invalid query
+ Query<int> emptyQuery{};
+ if (emptyQuery.isValid()){
+ RETURN_FAILURE("Empty query is not supposed to be valid." );
+ }
+ }
+ {
+ /// Test that the nullopt constructor does what it's supposed to do
+ Query<int>emptyQuery{std::nullopt};
+ if (emptyQuery.isValid()){
+ RETURN_FAILURE("std::nullopt query is not supposed to be valid." );
+ }
+ }
+
+ Query<int> initQuery{5};
+ if (!initQuery.isValid()){
+ RETURN_FAILURE("Query is supposed to be valid... And have value of 5." );
+ }
+ if (initQuery != 5) {
+ RETURN_FAILURE("Query is valid but has not 5 but instead "<<initQuery);
+ }
+ if (initQuery != 5) {
+ RETURN_FAILURE("Query is valid but has not 5 but instead "<<initQuery);
+ }
+ Query<int> copyQuery{initQuery};
+ if (!copyQuery.isValid()){
+ RETURN_FAILURE("The copy query is constructed from another valid one but is invalid...");
+ }
+ if (copyQuery.value_or(-9999) != initQuery.value_or(9999)){
+ RETURN_FAILURE("The copy query stores "<<copyQuery<<" while the other one has "<<initQuery);
+ }
+ copyQuery = std::nullopt;
+ if (copyQuery.isValid()){
+ RETURN_FAILURE("After setting back to nullopt it's supposed to be invalid." );
+ }
+ copyQuery = 7;
+ if (!copyQuery.isValid()){
+ RETURN_FAILURE("Expect the query to be valid." );
+ }
+ if (copyQuery != 7) {
+ RETURN_FAILURE("Expect the query to be 7 and not "<<copyQuery);
+ }
+ copyQuery = std::move(initQuery);
+ if (copyQuery != 5) {
+ RETURN_FAILURE("Expect the query to be 5 and not "<<copyQuery);
+ }
+ /// Apparently, the move on an optional does not invalidate the optional (https://stackoverflow.com/questions/51805059/why-does-moving-stdoptional-not-reset-state)
+ ///if (initQuery.isValid()) {
+ /// RETURN_FAILURE("Initial query needs to be invalid and not "<<initQuery);
+ ///}
+ initQuery = copyQuery;
+ if (!initQuery.isValid()) {
+ RETURN_FAILURE("Initial query needs to be valid again");
+ }
+ if (copyQuery.value_or(-9999) != initQuery.value_or(9999)){
+ RETURN_FAILURE("The copy query stores "<<copyQuery<<" while the other one has "<<initQuery);
+ }
+ std::cout<<__FILE__<<": - Test is passed. "<<std::endl;
+ return EXIT_SUCCESS;
+}
+
--
GitLab
From 14782434560175edc18dc592f08b1d5c9e5a0223 Mon Sep 17 00:00:00 2001
From: Johannes Josef Junggeburth <johannes.josef.junggeburth@cern.ch>
Date: Thu, 30 Jan 2025 17:24:00 +0100
Subject: [PATCH 2/5] Introduce GeoMaterial sorter helper struct
---
.../GeoModelHelpers/GeoMaterialSorter.h | 33 +++++++++++
.../GeoModelHelpers/src/GeoMaterialSorter.cxx | 56 +++++++++++++++++++
2 files changed, 89 insertions(+)
create mode 100644 GeoModelCore/GeoModelHelpers/GeoModelHelpers/GeoMaterialSorter.h
create mode 100644 GeoModelCore/GeoModelHelpers/src/GeoMaterialSorter.cxx
diff --git a/GeoModelCore/GeoModelHelpers/GeoModelHelpers/GeoMaterialSorter.h b/GeoModelCore/GeoModelHelpers/GeoModelHelpers/GeoMaterialSorter.h
new file mode 100644
index 000000000..2708ef3ce
--- /dev/null
+++ b/GeoModelCore/GeoModelHelpers/GeoModelHelpers/GeoMaterialSorter.h
@@ -0,0 +1,33 @@
+/*
+ Copyright (C) 2002-2024 CERN for the benefit of the ATLAS collaboration
+*/
+#ifndef GeoModelHelpers_GeoMaterialSorter_H
+#define GeoModelHelpers_GeoMaterialSorter_H
+
+#include "GeoModelKernel/GeoMaterial.h"
+#include "GeoModelKernel/GeoElement.h"
+#include "GeoModelKernel/GeoIntrusivePtr.h"
+#include <set>
+
+/// @brief Helper struct to deuplicate equivalent materials with different naming
+struct GeoMaterialSorter {
+ /** @brief Comparison operator returing whether Material A is smaller than Material B */
+ bool operator()(const GeoMaterial* a, const GeoMaterial* b) const;
+ /** @brief Comparison operator returing whether the Element A is smaller than Element B */
+ bool operator()(const GeoElement* a, const GeoElement* b) const;
+ /// @brief Compares 2 GeoMaterials
+ /// @param a : Pointer to material A
+ /// @param b : Pointer to material B
+ /// @return Returns 0 if no defining material property could be found that differs
+ /// Returns -1 if the first defining & differing property of A is smaller
+ /// Returns 1 otherwise
+ int compare(const GeoMaterial* a, const GeoMaterial* b) const;
+ int compare(const GeoElement* a, const GeoElement* b) const;
+
+};
+
+/// @brief
+/// @tparam ShapeType
+using GeoMaterialSet = std::set<GeoIntrusivePtr<const GeoMaterial>, GeoMaterialSorter>;
+
+#endif
\ No newline at end of file
diff --git a/GeoModelCore/GeoModelHelpers/src/GeoMaterialSorter.cxx b/GeoModelCore/GeoModelHelpers/src/GeoMaterialSorter.cxx
new file mode 100644
index 000000000..d1e00bc9a
--- /dev/null
+++ b/GeoModelCore/GeoModelHelpers/src/GeoMaterialSorter.cxx
@@ -0,0 +1,56 @@
+/*
+ Copyright (C) 2002-2025 CERN for the benefit of the ATLAS collaboration
+*/
+
+
+#include "GeoModelHelpers/GeoMaterialSorter.h"
+
+namespace {
+ // Tolerance in density to treat both material equally
+ constexpr double equivTol = 1.e-6;
+}
+
+
+bool GeoMaterialSorter::operator()(const GeoMaterial* a, const GeoMaterial* b) const{
+ return compare(a,b) < 0;
+}
+bool GeoMaterialSorter::operator()(const GeoElement* a, const GeoElement* b) const{
+ return compare(a,b) < 0;
+}
+
+int GeoMaterialSorter::compare(const GeoMaterial* a, const GeoMaterial* b) const {
+ if (a == b) {
+ return 0;
+ }
+ if (a->getNumElements() != b->getNumElements()) {
+ return a->getNumElements() < b->getNumElements();
+ }
+ const double densityCmp = a->getDensity() - b->getDensity();
+ if (std::abs(densityCmp) > equivTol) {
+ return densityCmp < 0. ? -1 : 1;
+ }
+ /// Assume sorting of elements by fraction
+ for (unsigned e = 0; e < a->getNumElements(); ++e) {
+ const double fracComp = a->getFraction(e) - b->getFraction(e);
+ if (std::abs(fracComp) > equivTol) {
+ return fracComp < 0.? -1: 1;
+ }
+ const int eleCmp = compare(a->getElement(e), b->getElement(e));
+ if (eleCmp) {
+ return eleCmp;
+ }
+ }
+ return 0;
+}
+int GeoMaterialSorter::compare(const GeoElement* a, const GeoElement* b) const {
+ if (a == b) {
+ return 0;
+ }
+ if (a->getZ() != b->getZ()) {
+ return a->getZ() < b->getZ() ? -1 : 1;
+ }
+ if (a->getA() != b->getA()) {
+ return a->getA() < b->getA() ? -1 : 1;
+ }
+ return 0;
+}
\ No newline at end of file
--
GitLab
From 930ed375799df1d24e2a13b927cd1ace459d5699 Mon Sep 17 00:00:00 2001
From: Johannes Josef Junggeburth <johannes.josef.junggeburth@cern.ch>
Date: Thu, 30 Jan 2025 17:25:27 +0100
Subject: [PATCH 3/5] GeoModelKernel - Tidy the GeoAccessVolumeAction & minor
tidy in the GeoMaterial & GeoVolumeCursor
---
.../GeoModelKernel/GeoAccessVolumeAction.h | 39 ++++++++-----------
.../GeoModelKernel/GeoMaterial.h | 29 +++++---------
.../GeoModelKernel/GeoNodeAction.h | 12 +++---
.../src/GeoAccessVolumeAction.cxx | 34 +++++-----------
.../GeoModelKernel/src/GeoNodeAction.cxx | 16 ++------
.../GeoModelKernel/src/GeoVolumeCursor.cxx | 13 +++----
6 files changed, 48 insertions(+), 95 deletions(-)
diff --git a/GeoModelCore/GeoModelKernel/GeoModelKernel/GeoAccessVolumeAction.h b/GeoModelCore/GeoModelKernel/GeoModelKernel/GeoAccessVolumeAction.h
index 2399f4c4a..113568fd1 100644
--- a/GeoModelCore/GeoModelKernel/GeoModelKernel/GeoAccessVolumeAction.h
+++ b/GeoModelCore/GeoModelKernel/GeoModelKernel/GeoAccessVolumeAction.h
@@ -1,5 +1,5 @@
/*
- Copyright (C) 2002-2024 CERN for the benefit of the ATLAS collaboration
+ Copyright (C) 2002-2025 CERN for the benefit of the ATLAS collaboration
*/
#ifndef GeoAccessVolumeAction_h
@@ -25,7 +25,7 @@ class GeoAccessVolumeAction final : public GeoNodeAction
public:
GeoAccessVolumeAction (unsigned int Index, const GeoVAlignmentStore* store);
- virtual ~GeoAccessVolumeAction() override;
+ virtual ~GeoAccessVolumeAction() =default;
/// Handles a Transform.
virtual void handleTransform (const GeoTransform *xform) override;
@@ -70,49 +70,42 @@ class GeoAccessVolumeAction final : public GeoNodeAction
virtual void handleSerialIdentifier(const GeoSerialIdentifier *sI) override;
private:
-
- GeoAccessVolumeAction(const GeoAccessVolumeAction &right);
- GeoAccessVolumeAction & operator=(const GeoAccessVolumeAction &right);
-
/// Returns a pointer to the ith physical volume under this one.
- PVConstLink m_volume;
+ PVConstLink m_volume{};
/// The transformation to the ith volume.
- GeoTrf::Transform3D m_transform;
+ GeoTrf::Transform3D m_transform{GeoTrf::Transform3D::Identity()};
/// The default transformation to the ith volume.
- GeoTrf::Transform3D m_defTransform;
+ GeoTrf::Transform3D m_defTransform{GeoTrf::Transform3D::Identity()};
/// The volume which we are interested in seeking.
- unsigned int m_index;
+ unsigned int m_index{0};
/// The volume which we are interested in seeking.
- unsigned int m_counter;
+ unsigned int m_counter{0};
/// The name of the volume. From a nametag or a serial denominator.
- mutable std::string m_name;
-
- /// The identifier of the volume. From an identifier tag.
- mutable Query<int> m_id;
+ mutable std::string m_name{};
/// A pointer to a name tag. If the volume is named.
- const GeoNameTag *m_nameTag;
+ const GeoNameTag *m_nameTag{nullptr};
/// A pointer to a serial denominator. If one exists.
- const GeoSerialDenominator *m_serialDenominator;
+ const GeoSerialDenominator *m_serialDenominator{nullptr};
/// A pointer to an identifier tag. If the volume is identified.
- const GeoIdentifierTag *m_idTag;
+ const GeoIdentifierTag *m_idTag{nullptr};
/// List of Pending Transformations.
- std::vector<const GeoTransform *> m_pendingTransformList;
+ std::vector<const GeoTransform *> m_pendingTransformList{};
/// Position of the serial denominator. Used to assign a numeric suffix to the name, eg BaseName+99
- unsigned int m_serialDenomPosition;
+ unsigned int m_serialDenomPosition{0};
- const GeoSerialIdentifier *m_serialIdentifier;
- unsigned int m_serialIdentPosition;
- const GeoVAlignmentStore* m_alignStore;
+ const GeoSerialIdentifier *m_serialIdentifier{nullptr};
+ unsigned int m_serialIdentPosition{0};
+ const GeoVAlignmentStore* m_alignStore{nullptr};
};
#endif
diff --git a/GeoModelCore/GeoModelKernel/GeoModelKernel/GeoMaterial.h b/GeoModelCore/GeoModelKernel/GeoModelKernel/GeoMaterial.h
index 8050cbf9f..907a7f7b7 100644
--- a/GeoModelCore/GeoModelKernel/GeoModelKernel/GeoMaterial.h
+++ b/GeoModelCore/GeoModelKernel/GeoModelKernel/GeoMaterial.h
@@ -74,14 +74,18 @@ class GeoMaterial : public RCBase {
double getFraction (int i) const;
// The name of the material.
- const std::string& getName () const;
-
+ const std::string& getName () const{
+ return m_name;
+ }
// The density of the material.
- const double& getDensity () const;
-
+ double getDensity () const{
+ return m_density;
+ }
// Gives an integral identifier for the material. For
// convenience.
- const unsigned int& getID () const;
+ unsigned int getID () const {
+ return m_iD;
+ }
protected:
virtual ~GeoMaterial() = default;
@@ -121,19 +125,4 @@ class GeoMaterial : public RCBase {
};
-inline const std::string& GeoMaterial::getName () const
-{
- return m_name;
-}
-
-inline const double& GeoMaterial::getDensity () const
-{
- return m_density;
-}
-
-inline const unsigned int& GeoMaterial::getID () const
-{
- return m_iD;
-}
-
#endif
diff --git a/GeoModelCore/GeoModelKernel/GeoModelKernel/GeoNodeAction.h b/GeoModelCore/GeoModelKernel/GeoModelKernel/GeoNodeAction.h
index ba80f9198..a26100d86 100644
--- a/GeoModelCore/GeoModelKernel/GeoModelKernel/GeoNodeAction.h
+++ b/GeoModelCore/GeoModelKernel/GeoModelKernel/GeoNodeAction.h
@@ -1,5 +1,5 @@
/*
- Copyright (C) 2002-2024 CERN for the benefit of the ATLAS collaboration
+ Copyright (C) 2002-2025 CERN for the benefit of the ATLAS collaboration
*/
#ifndef GEOMODELKERNEL_GEONODEACTION_H
@@ -40,10 +40,10 @@ class GeoNodeAction
{ ALL_ANCESTORS = -1, SELF = 0, SELF_AND_CHILDREN = 1 };
public:
- GeoNodeAction();
+ GeoNodeAction() = default;
GeoNodeAction(const GeoNodeAction &right) = delete;
GeoNodeAction & operator=(const GeoNodeAction &right) = delete;
- virtual ~GeoNodeAction();
+ virtual ~GeoNodeAction() = default;
// Handles a Node.
virtual void handleNode (const GeoGraphNode *);
@@ -109,15 +109,15 @@ class GeoNodeAction
protected:
// Termination flag; causes an abortion of action execution.
- bool m_terminate;
+ bool m_terminate{false};
private:
// A limit may be placed upon the depth to which the action
// descends. 0 = self. 1 = self and children.
- Query<unsigned int> m_depth;
+ Query<unsigned int> m_depth{};
- GeoNodePath m_path;
+ GeoNodePath m_path{};
};
#endif
diff --git a/GeoModelCore/GeoModelKernel/src/GeoAccessVolumeAction.cxx b/GeoModelCore/GeoModelKernel/src/GeoAccessVolumeAction.cxx
index 82bf0cc65..95c969fd4 100755
--- a/GeoModelCore/GeoModelKernel/src/GeoAccessVolumeAction.cxx
+++ b/GeoModelCore/GeoModelKernel/src/GeoAccessVolumeAction.cxx
@@ -1,5 +1,5 @@
/*
- Copyright (C) 2002-2024 CERN for the benefit of the ATLAS collaboration
+ Copyright (C) 2002-2025 CERN for the benefit of the ATLAS collaboration
*/
#include "GeoModelKernel/GeoAccessVolumeAction.h"
@@ -9,26 +9,13 @@
#include <string>
-GeoAccessVolumeAction::GeoAccessVolumeAction(unsigned int Index, const GeoVAlignmentStore* store)
- : m_transform(GeoTrf::Transform3D::Identity())
- , m_defTransform(GeoTrf::Transform3D::Identity())
- , m_index(Index)
- , m_counter(0)
- , m_nameTag(nullptr)
- , m_serialDenominator(nullptr)
- , m_idTag(nullptr)
- , m_serialDenomPosition(0)
- , m_serialIdentifier(nullptr)
- , m_serialIdentPosition(0)
- , m_alignStore(store)
-{
+GeoAccessVolumeAction::GeoAccessVolumeAction(unsigned int Index, const GeoVAlignmentStore* store):
+ m_index(Index),
+ m_alignStore(store) {
setDepthLimit (1);
m_pendingTransformList.reserve(1);
}
-GeoAccessVolumeAction::~GeoAccessVolumeAction()
-{
-}
void GeoAccessVolumeAction::handleTransform (const GeoTransform *xform)
{
@@ -137,8 +124,7 @@ const GeoTrf::Transform3D & GeoAccessVolumeAction::getDefTransform () const
return m_defTransform;
}
-const std::string & GeoAccessVolumeAction::getName () const
-{
+const std::string & GeoAccessVolumeAction::getName () const {
if(m_name.empty()) {
if(m_nameTag) {
m_name = m_nameTag->getName();
@@ -199,16 +185,14 @@ void GeoAccessVolumeAction::handleIdentifierTag (const GeoIdentifierTag *idTag)
m_serialIdentPosition = 0;
}
-Query<int> GeoAccessVolumeAction::getId () const
-{
- Query<int> id;
+Query<int> GeoAccessVolumeAction::getId () const {
if(m_idTag) {
- id = Query<int>(m_idTag->getIdentifier());
+ return Query<int>(m_idTag->getIdentifier());
}
else if(m_serialIdentifier) {
- id = Query<int>(m_index - m_serialIdentPosition + m_serialIdentifier->getBaseId());
+ return Query<int>(m_index - m_serialIdentPosition + m_serialIdentifier->getBaseId());
}
- return id;
+ return std::nullopt;
}
void GeoAccessVolumeAction::handleSerialIdentifier(const GeoSerialIdentifier *sI)
diff --git a/GeoModelCore/GeoModelKernel/src/GeoNodeAction.cxx b/GeoModelCore/GeoModelKernel/src/GeoNodeAction.cxx
index 7732f697c..c0aced898 100755
--- a/GeoModelCore/GeoModelKernel/src/GeoNodeAction.cxx
+++ b/GeoModelCore/GeoModelKernel/src/GeoNodeAction.cxx
@@ -1,18 +1,9 @@
/*
- Copyright (C) 2002-2024 CERN for the benefit of the ATLAS collaboration
+ Copyright (C) 2002-2025 CERN for the benefit of the ATLAS collaboration
*/
#include "GeoModelKernel/GeoNodeAction.h"
-GeoNodeAction::GeoNodeAction()
- : m_terminate (false)
-{
-}
-
-GeoNodeAction::~GeoNodeAction()
-{
-}
-
void GeoNodeAction::handleNode (const GeoGraphNode *)
{
// Do nothing...
@@ -64,9 +55,8 @@ void GeoNodeAction::setDepthLimit (unsigned int limit)
m_depth = limit;
}
-void GeoNodeAction::clearDepthLimit ()
-{
- m_depth = Query < unsigned int >();
+void GeoNodeAction::clearDepthLimit () {
+ m_depth = std::nullopt;
}
void GeoNodeAction::handleSerialTransformer (const GeoSerialTransformer *)
diff --git a/GeoModelCore/GeoModelKernel/src/GeoVolumeCursor.cxx b/GeoModelCore/GeoModelKernel/src/GeoVolumeCursor.cxx
index d28c9d1c1..c17850808 100755
--- a/GeoModelCore/GeoModelKernel/src/GeoVolumeCursor.cxx
+++ b/GeoModelCore/GeoModelKernel/src/GeoVolumeCursor.cxx
@@ -255,19 +255,16 @@ std::string GeoVolumeCursor::getName () const
return name;
}
-Query<int> GeoVolumeCursor::getId () const
-{
- Query<int> id;
+Query<int> GeoVolumeCursor::getId () const {
if (m_idTag) {
- id = Query<int> (m_idTag->getIdentifier ());
+ return Query<int>{m_idTag->getIdentifier ()};
}
else if (m_serialIdentifier) {
- id = Query<int> (m_volCount - m_serialIdentPosition - 1 + m_serialIdentifier->getBaseId());
+ return Query<int>{m_volCount - m_serialIdentPosition - 1 + m_serialIdentifier->getBaseId()};
}
- return id;
+ return std::nullopt;
}
-bool GeoVolumeCursor::hasAlignableTransform() const
-{
+bool GeoVolumeCursor::hasAlignableTransform() const {
return m_hasAlignTrans;
}
--
GitLab
From ae4944be7d62f6026783932c29858fb33d88850a Mon Sep 17 00:00:00 2001
From: Johannes Josef Junggeburth <johannes.josef.junggeburth@cern.ch>
Date: Thu, 30 Jan 2025 17:26:15 +0100
Subject: [PATCH 4/5] Add volumeId to the GeoChildNodeWithTrf
---
.../GeoModelHelpers/GeoModelHelpers/getChildNodesWithTrf.h | 6 ++++--
GeoModelCore/GeoModelHelpers/src/getChildNodesWithTrf.cxx | 7 ++++---
2 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/GeoModelCore/GeoModelHelpers/GeoModelHelpers/getChildNodesWithTrf.h b/GeoModelCore/GeoModelHelpers/GeoModelHelpers/getChildNodesWithTrf.h
index 413646006..6c9fa930a 100644
--- a/GeoModelCore/GeoModelHelpers/GeoModelHelpers/getChildNodesWithTrf.h
+++ b/GeoModelCore/GeoModelHelpers/GeoModelHelpers/getChildNodesWithTrf.h
@@ -7,6 +7,7 @@
#include "GeoModelKernel/GeoVPhysVol.h"
#include "GeoModelKernel/GeoDefinitions.h"
+#include <optional>
class GeoVolumeCursor;
@@ -27,9 +28,10 @@ struct GeoChildNodeWithTrf {
bool isAlignable{false};
/// @brief tag whether the physical volume is a full physVol
bool isSensitive{false};
-
+ /// @brief Identifier of the volume. If there's any and no copies are active
+ std::optional<int> volumeId{std::nullopt};
GeoChildNodeWithTrf() = default;
- GeoChildNodeWithTrf(GeoVolumeCursor& curs);
+ GeoChildNodeWithTrf(GeoVolumeCursor& curs);
};
/*** @brief Returns all direct children of a volume with their transform. Equicalent volumes can be summarized.
diff --git a/GeoModelCore/GeoModelHelpers/src/getChildNodesWithTrf.cxx b/GeoModelCore/GeoModelHelpers/src/getChildNodesWithTrf.cxx
index cdfdce072..f1d8fec4b 100644
--- a/GeoModelCore/GeoModelHelpers/src/getChildNodesWithTrf.cxx
+++ b/GeoModelCore/GeoModelHelpers/src/getChildNodesWithTrf.cxx
@@ -1,5 +1,5 @@
/*
- Copyright (C) 2002-2024 CERN for the benefit of the ATLAS collaboration
+ Copyright (C) 2002-2025 CERN for the benefit of the ATLAS collaboration
*/
#include <utility>
@@ -20,11 +20,11 @@ namespace {
volume{curs.getVolume()},
nodeName{curs.getName()},
isAlignable{curs.hasAlignableTransform()},
- isSensitive{typeid(*volume) == typeid(GeoFullPhysVol)} {
+ isSensitive{typeid(*volume) == typeid(GeoFullPhysVol)},
+ volumeId{static_cast<const std::optional<int>&>(curs.getId())} {
//// Do not specify a node name if it's a dummy one
if (nodeName == dummyNodeName) {
nodeName = volume->getLogVol()->getName();
-
}
}
@@ -53,6 +53,7 @@ std::vector <GeoChildNodeWithTrf> getChildrenWithRef(PVConstLink physVol,
children.emplace_back(std::move(currentChild));
} else if (prevChild.nCopies == 1) {
++prevChild.nCopies;
+ prevChild.volumeId = std::nullopt;
prevChild.inductionRule = prevChild.transform.inverse() *
currentChild.transform;
} else if (!transSort.compare(prevChild.inductionRule,
--
GitLab
From 79d85ed610ac266b7e3478f90f89778cc477d38d Mon Sep 17 00:00:00 2001
From: Johannes Josef Junggeburth <johannes.josef.junggeburth@cern.ch>
Date: Thu, 30 Jan 2025 17:26:46 +0100
Subject: [PATCH 5/5] Minor tidy in variantHelpers / defineWorld
---
.../GeoModelHelpers/variantHelpers.h | 74 ++++++++-----------
.../GeoModelHelpers/src/defineWorld.cxx | 20 +++--
.../GMSTATISTICS/src/gmstatistics.cxx | 28 +++----
3 files changed, 52 insertions(+), 70 deletions(-)
diff --git a/GeoModelCore/GeoModelHelpers/GeoModelHelpers/variantHelpers.h b/GeoModelCore/GeoModelHelpers/GeoModelHelpers/variantHelpers.h
index ec6702d98..d0808cbba 100644
--- a/GeoModelCore/GeoModelHelpers/GeoModelHelpers/variantHelpers.h
+++ b/GeoModelCore/GeoModelHelpers/GeoModelHelpers/variantHelpers.h
@@ -1,4 +1,4 @@
-// Copyright (C) 2002-2024 CERN for the benefit of the ATLAS collaboration
+// Copyright (C) 2002-2025 CERN for the benefit of the ATLAS collaboration
/*
* This header file provides helper C++ functions used in GeoModel IO code.
@@ -23,12 +23,10 @@
namespace GeoModelHelpers {
- class variantHelper
- {
+ class variantHelper {
public:
-
- static void printStdVectorVariants(const std::vector<std::variant<int, long, float, double, std::string>> vec)
- {
+ using VariantType_t = std::variant<int, long, float, double, std::string>;
+ static void printStdVectorVariants(const std::vector<VariantType_t>& vec) {
for (const auto &item : vec)
{
if (std::holds_alternative<int>(item))
@@ -47,71 +45,57 @@ namespace GeoModelHelpers {
std::cout << std::endl;
}
- static std::string getFromVariant_String(const std::variant<int, long, float, double, std::string> &record, std::string_view logMsg = "")
- {
- std::string_view type{"string"};
+ static std::string getFromVariant_String(const VariantType_t &record, std::string_view logMsg = "") {
+ constexpr std::string_view type{"string"};
std::string ret;
- try
- {
+ try {
ret = std::get<std::string>(record);
- }
- catch (std::bad_variant_access const &ex)
- {
- THROW_EXCEPTION(std::string(ex.what()) + ": '" + std::string(logMsg) + "' is not a '" + std::string(type) + "'! It's a '" + getFromVariant_Type(record) + "'.");
+ } catch (std::bad_variant_access const &ex){
+ THROW_EXCEPTION(ex.what()<<": '"<<logMsg<<"' is not a '"<<type<<"'! It's a '"
+ <<getFromVariant_Type(record)<<"'.");
}
return ret;
}
- static int getFromVariant_Int(const std::variant<int, long, float, double, std::string> &record, std::string_view logMsg = "")
- {
+ static int getFromVariant_Int(const VariantType_t &record, std::string_view logMsg = "") {
std::string_view type{"int"};
- int ret;
+ int ret{0};
- try
- {
+ try {
ret = std::get<int>(record);
}
- catch (std::bad_variant_access const &ex)
- {
- THROW_EXCEPTION(std::string(ex.what()) + ": '" + std::string(logMsg) + "' is not a '" + std::string(type) + "'! It's a '" + getFromVariant_Type(record) + "'.");
-
+ catch (std::bad_variant_access const &ex) {
+ THROW_EXCEPTION(ex.what()<<": '"<<logMsg<<"' is not a '"<<type<<"'! It's a '"
+ <<getFromVariant_Type(record)<<"'.");
}
return ret;
}
- static double getFromVariant_Double(const std::variant<int, long, float, double, std::string> &record, std::string_view logMsg = "")
- {
- std::string_view type{"double"};
- double ret;
- try
- {
+ static double getFromVariant_Double(const VariantType_t &record, std::string_view logMsg = "") {
+ constexpr std::string_view type{"double"};
+ double ret{0.};
+ try {
ret = std::get<double>(record);
}
- catch (std::bad_variant_access const &ex)
- {
- THROW_EXCEPTION(std::string(ex.what()) + ": '" + std::string(logMsg) + "' is not a '" + std::string(type) + "'! It's a '" + getFromVariant_Type(record) + "'.");
+ catch (std::bad_variant_access const &ex) {
+ THROW_EXCEPTION(ex.what()<<": '"<<logMsg<<"' is not a '"<<type<<"'! It's a '"
+ <<getFromVariant_Type(record)<<"'.");
}
return ret;
}
- static std::string getFromVariant_Type(const std::variant<int, long, float, double, std::string> &record)
- {
+ static std::string getFromVariant_Type(const VariantType_t &record) {
std::string type;
- if (std::holds_alternative<int>(record))
- {
+ if (std::holds_alternative<int>(record)) {
type = "int";
}
- else if (std::holds_alternative<long>(record))
- {
+ else if (std::holds_alternative<long>(record)) {
type = "long";
}
- else if (std::holds_alternative<float>(record))
- {
+ else if (std::holds_alternative<float>(record)) {
type = "float";
}
- else if (std::holds_alternative<double>(record))
- {
+ else if (std::holds_alternative<double>(record)){
type = "double";
}
- else if (std::holds_alternative<std::string>(record))
- {
+ else if (std::holds_alternative<std::string>(record)) {
type = "string";
} else {
type = "UNKOWN";
diff --git a/GeoModelCore/GeoModelHelpers/src/defineWorld.cxx b/GeoModelCore/GeoModelHelpers/src/defineWorld.cxx
index 2dcff72d1..8d6d137ba 100644
--- a/GeoModelCore/GeoModelHelpers/src/defineWorld.cxx
+++ b/GeoModelCore/GeoModelHelpers/src/defineWorld.cxx
@@ -1,5 +1,5 @@
/*
- Copyright (C) 2002-2024 CERN for the benefit of the ATLAS collaboration
+ Copyright (C) 2002-2025 CERN for the benefit of the ATLAS collaboration
*/
#include "GeoModelHelpers/defineWorld.h"
#include "GeoModelHelpers/cloneVolume.h"
@@ -8,8 +8,6 @@
#include "GeoModelKernel/GeoBox.h"
#include "GeoModelKernel/GeoPhysVol.h"
#include "GeoModelKernel/GeoFullPhysVol.h"
-#include "GeoModelKernel/GeoVolumeCursor.h"
-#include "GeoModelKernel/GeoAlignableTransform.h"
#include "GeoModelKernel/Units.h"
GeoIntrusivePtr<GeoPhysVol> createGeoWorld(const double worldBoxX,
@@ -22,13 +20,13 @@ GeoIntrusivePtr<GeoPhysVol> createGeoWorld(const double worldBoxX,
constexpr double cm3 = GeoModelKernelUnits::cm3;
// Define the chemical elements
- GeoIntrusivePtr<GeoElement> Nitrogen{new GeoElement ("Nitrogen" ,"N" , 7.0 , 14.0031 *gr/mole)};
- GeoIntrusivePtr<GeoElement> Oxygen{new GeoElement ("Oxygen" ,"O" , 8.0 , 15.9949 *gr/mole)};
- GeoIntrusivePtr<GeoElement> Argon{new GeoElement ("Argon" ,"Ar" , 18.0 , 39.9624 *gr/mole)};
- GeoIntrusivePtr<GeoElement> Hydrogen{new GeoElement ("Hydrogen" ,"H" , 1.0 , 1.00782503081372 *gr/mole)};
+ GeoIntrusivePtr<GeoElement> Nitrogen{make_intrusive<GeoElement>("Nitrogen" ,"N" , 7.0 , 14.0031 *gr/mole)};
+ GeoIntrusivePtr<GeoElement> Oxygen{make_intrusive<GeoElement>("Oxygen" ,"O" , 8.0 , 15.9949 *gr/mole)};
+ GeoIntrusivePtr<GeoElement> Argon{make_intrusive<GeoElement>("Argon" ,"Ar" , 18.0 , 39.9624 *gr/mole)};
+ GeoIntrusivePtr<GeoElement> Hydrogen{make_intrusive<GeoElement>("Hydrogen" ,"H" , 1.0 , 1.00782503081372 *gr/mole)};
constexpr double densityOfAir=0.001290 *gr/cm3;
- GeoIntrusivePtr<GeoMaterial> air{new GeoMaterial("GeoModelAir", densityOfAir)};
+ GeoIntrusivePtr<GeoMaterial> air{make_intrusive<GeoMaterial>("GeoModelAir", densityOfAir)};
air->add(Nitrogen , 0.7494);
air->add(Oxygen, 0.2369);
air->add(Argon, 0.0129);
@@ -36,9 +34,9 @@ GeoIntrusivePtr<GeoPhysVol> createGeoWorld(const double worldBoxX,
air->lock();
- GeoIntrusivePtr<GeoBox> worldBox{new GeoBox(worldBoxX, worldBoxY, worldBoxZ)};
- GeoIntrusivePtr<GeoLogVol> worldLog{new GeoLogVol("WorldLog", worldBox, air)};
- GeoIntrusivePtr<GeoPhysVol> world{new GeoPhysVol(worldLog)};
+ GeoIntrusivePtr<GeoBox> worldBox{make_intrusive<GeoBox>(worldBoxX, worldBoxY, worldBoxZ)};
+ GeoIntrusivePtr<GeoLogVol> worldLog{make_intrusive<GeoLogVol>("WorldLog", worldBox, air)};
+ GeoIntrusivePtr<GeoPhysVol> world{make_intrusive<GeoPhysVol>(worldLog)};
return world;
}
diff --git a/GeoModelTools/GMSTATISTICS/src/gmstatistics.cxx b/GeoModelTools/GMSTATISTICS/src/gmstatistics.cxx
index 1dac430b4..dbafd01bd 100644
--- a/GeoModelTools/GMSTATISTICS/src/gmstatistics.cxx
+++ b/GeoModelTools/GMSTATISTICS/src/gmstatistics.cxx
@@ -146,28 +146,28 @@ int main(int argc, char ** argv) {
GeoGeometryPluginLoader loader;
GeoVGeometryPlugin *factory=loader.load(plugin);
if (!factory) {
- std::cerr << "Could not load plugin " << plugin << std::endl;
- std::cout.rdbuf(coutBuff);
- return 5;
+ std::cerr << "Could not load plugin " << plugin << std::endl;
+ std::cout.rdbuf(coutBuff);
+ return 5;
}
unsigned int expand{0};
unsigned int net{0};
{
- GeoIntrusivePtr<GeoVPhysVol> world{createGeoWorld()};
+ GeoIntrusivePtr<GeoVPhysVol> world{createGeoWorld()};
- int before=snoop();
- factory->create(world);
- net=snoop()-before;
- std::cout.rdbuf(coutBuff);
+ int before=snoop();
+ factory->create(world);
+ net=snoop()-before;
+ std::cout.rdbuf(coutBuff);
- if (printTree) {
- GeoInventoryGraphAction action(std::cout);
- world->exec(&action);
- }
- expand=heapsize();
+ if (printTree) {
+ GeoInventoryGraphAction action(std::cout);
+ world->exec(&action);
+ }
+ expand=heapsize();
}
unsigned int contract=expand-heapsize();
- std::cout << basename((char *) plugin.c_str()) << " allocates " << net/factor << " MB" << " net GeoModel " << contract/1000000.0 << " MB" << std::endl;
+ std::cout <<factory->getName() << " allocates " << net/factor << " MB" << " net GeoModel " << contract/1000000.0 << " MB" << std::endl;
delete factory;
std::cout.rdbuf(fileBuff);
}
--
GitLab