complete actual copy implementation

Core/include/Acts/EventData/MultiTrajectory.hpp

@@ -1525,7 +1525,7 @@ class MultiTrajectory {
  private:
   template <typename T, bool RO = ReadOnly, typename = std::enable_if_t<!RO>>
   void copyDynamicFrom(IndexType dstIdx, const T& src, IndexType srcIdx) {
-    auto dynamicKeys = src.self().dynamicKeys_impl();
+    const auto& dynamicKeys = src.self().dynamicKeys_impl();
     for (const auto key : dynamicKeys) {
       std::any srcPtr = src.self().component_impl(key, srcIdx);
       self().copyDynamicFrom_impl(dstIdx, key, srcPtr);

Core/include/Acts/EventData/TrackContainer.hpp

@@ -315,7 +315,7 @@ class TrackContainer {
  private:
   template <typename T, bool RO = ReadOnly, typename = std::enable_if_t<!RO>>
   void copyDynamicFrom(IndexType dstIdx, const T& src, IndexType srcIdx) {
-    auto dynamicKeys = src.dynamicKeys_impl();
+    const auto& dynamicKeys = src.dynamicKeys_impl();
     for (const auto key : dynamicKeys) {
       std::any srcPtr = src.component_impl(key, srcIdx);
       container().copyDynamicFrom_impl(dstIdx, key, srcPtr);

Core/include/Acts/EventData/VectorMultiTrajectory.hpp

@@ -181,6 +181,7 @@ class VectorMultiTrajectoryBase {
     for (const auto& [key, value] : other.m_dynamic) {
       m_dynamic.insert({key, value->clone()});
     }
+    m_dynamicKeys = other.m_dynamicKeys;
   };
 
   VectorMultiTrajectoryBase(VectorMultiTrajectoryBase&& other) = default;
@@ -289,16 +290,8 @@ class VectorMultiTrajectoryBase {
   }
 
  public:
-  // @TODO: Reconsider return type, by-value -> expensive
-  // Could be generic iterator pair, possibly driven by iterator adapter that
-  // unpacks from unordered_map dynamically
-  std::vector<Acts::HashedString> dynamicKeys_impl() const {
-    std::vector<Acts::HashedString> result;
-    result.reserve(m_dynamic.size());
-    for (const auto& [key, value] : m_dynamic) {
-      result.push_back(key);
-    }
-    return result;
+  const std::vector<Acts::HashedString>& dynamicKeys_impl() const {
+    return m_dynamicKeys;
   }
 
   // END INTERFACE HELPER
@@ -340,6 +333,7 @@ class VectorMultiTrajectoryBase {
   // be handled in a smart way by moving but not sure.
   std::vector<std::shared_ptr<const Surface>> m_referenceSurfaces;
 
+  std::vector<HashedString> m_dynamicKeys;
   std::unordered_map<HashedString, std::unique_ptr<detail::DynamicColumnBase>>
       m_dynamic;
 };
@@ -459,8 +453,9 @@ class VectorMultiTrajectory final
 
   template <typename T>
   constexpr void addColumn_impl(const std::string& key) {
-    m_dynamic.insert(
-        {hashString(key), std::make_unique<detail::DynamicColumn<T>>()});
+    Acts::HashedString hashedKey = hashString(key);
+    m_dynamic.insert({hashedKey, std::make_unique<detail::DynamicColumn<T>>()});
+    m_dynamicKeys.push_back(hashedKey);
   }
 
   constexpr bool hasColumn_impl(HashedString key) const {

Core/include/Acts/EventData/VectorTrackContainer.hpp

@@ -165,16 +165,8 @@ class VectorTrackContainerBase {
     return m_tipIndex.size();
   }
 
-  // @TODO: Reconsider return type, by-value -> expensive
-  // Could be generic iterator pair, possibly driven by iterator adapter that
-  // unpacks from unordered_map dynamically
-  std::vector<Acts::HashedString> dynamicKeys_impl() const {
-    std::vector<Acts::HashedString> result;
-    result.reserve(m_dynamic.size());
-    for (const auto& [key, value] : m_dynamic) {
-      result.push_back(key);
-    }
-    return result;
+  const std::vector<Acts::HashedString>& dynamicKeys_impl() const {
+    return m_dynamicKeys;
   }
 
   // END INTERFACE HELPER
@@ -193,6 +185,7 @@ class VectorTrackContainerBase {
   std::vector<unsigned int> m_nOutliers;
   std::vector<unsigned int> m_nSharedHits;
 
+  std::vector<HashedString> m_dynamicKeys;
   std::unordered_map<HashedString, std::unique_ptr<detail::DynamicColumnBase>>
       m_dynamic;
 };
@@ -232,8 +225,9 @@ class VectorTrackContainer final : public detail_vtc::VectorTrackContainerBase {
 
   template <typename T>
   constexpr void addColumn_impl(const std::string& key) {
-    m_dynamic.insert(
-        {hashString(key), std::make_unique<detail::DynamicColumn<T>>()});
+    Acts::HashedString hashedKey = hashString(key);
+    m_dynamic.insert({hashedKey, std::make_unique<detail::DynamicColumn<T>>()});
+    m_dynamicKeys.push_back(hashedKey);
   }
 
   Parameters parameters(IndexType itrack) {

Plugins/Podio/include/Acts/Plugins/Podio/PodioTrackContainer.hpp

@@ -115,13 +115,8 @@ class PodioTrackContainerBase {
   }
 
   template <typename T>
-  static std::vector<Acts::HashedString> dynamicKeys_impl(T& instance) {
-    std::vector<Acts::HashedString> result;
-    result.reserve(instance.m_dynamic.size());
-    for (const auto& [key, value] : instance.m_dynamic) {
-      result.push_back(key);
-    }
-    return result;
+  static const std::vector<Acts::HashedString>& dynamicKeys_impl(T& instance) {
+    return instance.m_dynamicKeys;
   }
 
   template <typename T>
@@ -223,8 +218,10 @@ class MutablePodioTrackContainer : public PodioTrackContainerBase {
 
   template <typename T>
   constexpr void addColumn_impl(const std::string& key) {
-    m_dynamic.insert({hashString(key),
-                      std::make_unique<podio_detail::DynamicColumn<T>>(key)});
+    Acts::HashedString hashedKey = hashString(key);
+    m_dynamic.insert(
+        {hashedKey, std::make_unique<podio_detail::DynamicColumn<T>>(key)});
+    m_dynamicKeys.push_back(hashedKey);
   }
 
   Parameters parameters(IndexType itrack) {
@@ -275,7 +272,7 @@ class MutablePodioTrackContainer : public PodioTrackContainerBase {
     }
   }
 
-  std::vector<Acts::HashedString> dynamicKeys_impl() const {
+  const std::vector<Acts::HashedString>& dynamicKeys_impl() const {
     return PodioTrackContainerBase::dynamicKeys_impl(*this);
   }
 
@@ -294,6 +291,7 @@ class MutablePodioTrackContainer : public PodioTrackContainerBase {
   friend PodioTrackContainerBase;
 
   std::unique_ptr<ActsPodioEdm::TrackCollection> m_collection;
+  std::vector<HashedString> m_dynamicKeys;
   std::unordered_map<HashedString,
                      std::unique_ptr<podio_detail::DynamicColumnBase>>
       m_dynamic;
@@ -306,6 +304,7 @@ class ConstPodioTrackContainer : public PodioTrackContainerBase {
   ConstPodioTrackContainer(const PodioUtil::ConversionHelper& helper,
                            const ActsPodioEdm::TrackCollection& collection)
       : PodioTrackContainerBase{helper}, m_collection{&collection} {
+    // Not much we can do to recover dynamic columns here
     populateSurfaceBuffer(m_helper, *m_collection, m_surfaces);
   }
 
@@ -336,8 +335,10 @@ class ConstPodioTrackContainer : public PodioTrackContainerBase {
     populateSurfaceBuffer(m_helper, *m_collection, m_surfaces);
 
     // let's find dynamic columns
-    using types =
-        std::tuple<int32_t, int64_t, uint32_t, uint64_t, float, double>;
+    // See
+    // https://github.com/AIDASoft/podio/blob/858c0ff0b841705d1b18aafd57569fcbd1beda91/include/podio/UserDataCollection.h#L30-L31
+    using types = std::tuple<float, double, int8_t, int16_t, int32_t, int64_t,
+                             uint8_t, uint16_t, uint32_t, uint64_t>;
 
     for (const auto& col : available) {
       std::string prefix = tracksKey + "_extra__";
@@ -375,7 +376,9 @@ class ConstPodioTrackContainer : public PodioTrackContainerBase {
                                  "' is not of allowed type"};
       }
 
-      m_dynamic.insert({hashString(dynName), std::move(up)});
+      HashedString hashedKey = hashString(dynName);
+      m_dynamic.insert({hashedKey, std::move(up)});
+      m_dynamicKeys.push_back(hashedKey);
     }
   }
 
@@ -422,6 +425,7 @@ class ConstPodioTrackContainer : public PodioTrackContainerBase {
   std::unordered_map<HashedString,
                      std::unique_ptr<podio_detail::ConstDynamicColumnBase>>
       m_dynamic;
+  std::vector<HashedString> m_dynamicKeys;
 };
 
 ACTS_STATIC_CHECK_CONCEPT(ConstTrackContainerBackend, ConstPodioTrackContainer);

Plugins/Podio/include/Acts/Plugins/Podio/PodioTrackStateContainer.hpp

@@ -294,7 +294,9 @@ class ConstPodioTrackStateContainer final
                                  "' is not of allowed type"};
       }
 
-      m_dynamic.insert({hashString(dynName), std::move(up)});
+      HashedString hashedKey = hashString(dynName);
+      m_dynamic.insert({hashedKey, std::move(up)});
+      m_dynamicKeys.push_back(hashedKey);
     }
   }
 
@@ -385,6 +387,7 @@ class ConstPodioTrackStateContainer final
   std::unordered_map<HashedString,
                      std::unique_ptr<podio_detail::ConstDynamicColumnBase>>
       m_dynamic;
+  std::vector<HashedString> m_dynamicKeys;
 };
 
 static_assert(IsReadOnlyMultiTrajectory<ConstPodioTrackStateContainer>::value,
@@ -616,8 +619,10 @@ class MutablePodioTrackStateContainer final
 
   template <typename T>
   constexpr void addColumn_impl(const std::string& key) {
-    m_dynamic.insert({hashString(key),
-                      std::make_unique<podio_detail::DynamicColumn<T>>(key)});
+    HashedString hashedKey = hashString(key);
+    m_dynamic.insert(
+        {hashedKey, std::make_unique<podio_detail::DynamicColumn<T>>(key)});
+    m_dynamicKeys.push_back(hashedKey);
   }
 
   void allocateCalibrated_impl(IndexType istate, std::size_t measdim) {
@@ -697,6 +702,7 @@ class MutablePodioTrackStateContainer final
   std::unordered_map<HashedString,
                      std::unique_ptr<podio_detail::DynamicColumnBase>>
       m_dynamic;
+  std::vector<HashedString> m_dynamicKeys;
 };
 
 static_assert(
@@ -709,30 +715,4 @@ static_assert(!MutablePodioTrackStateContainer::ReadOnly,
 ACTS_STATIC_CHECK_CONCEPT(MutableMultiTrajectoryBackend,
                           MutablePodioTrackStateContainer);
 
-// ConstPodioTrackStateContainer::ConstPodioTrackStateContainer(
-// MutablePodioTrackStateContainer&& other)
-// : m_helper{other.m_helper},
-// m_collection{std::move(other.m_collection)},
-// m_params{std::move(other.m_params)},
-// m_jacs{std::move(other.m_jacs)},
-// m_surfaces{std::move(other.m_surfaces)} {}
-
-// ConstPodioTrackStateContainer::ConstPodioTrackStateContainer(
-// const MutablePodioTrackStateContainer& other)
-// : m_helper{other.m_helper},
-// m_surfaces{other.m_surfaces.begin(), other.m_surfaces.end()} {
-// for (auto src : *other.m_collection) {
-// auto dst = m_collection->create();
-// dst = src.clone();
-// }
-// for (auto src : *other.m_params) {
-// auto dst = m_params->create();
-// dst = src.clone();
-// }
-// for (auto src : *other.m_jacs) {
-// auto dst = m_jacs->create();
-// dst = src.clone();
-// }
-// }
-
 }  // namespace Acts

Tests/UnitTests/Plugins/Podio/CMakeLists.txt

@@ -1,3 +1,3 @@
 set(unittest_extra_libraries ActsPluginPodio)
-add_unittest(ConvertTrackPodio ConvertTrackPodioTest.cpp)
+add_unittest(PodioTrackContainer PodioTrackContainerTest.cpp)
 add_unittest(PodioTrackStateContainer PodioTrackStateContainerTest.cpp)

Tests/UnitTests/Plugins/Podio/ConvertTrackPodioTest.cpp → Tests/UnitTests/Plugins/Podio/PodioTrackContainerTest.cpp

@@ -40,6 +40,7 @@
 #include "Acts/Surfaces/StrawSurface.hpp"
 #include "Acts/Surfaces/SurfaceBounds.hpp"
 #include "Acts/Utilities/Helpers.hpp"
+#include "Acts/Utilities/Zip.hpp"
 #include "ActsPodioEdm/Surface.h"
 #include <ActsPodioEdm/TrackCollection.h>
 
@@ -52,7 +53,7 @@
 using namespace Acts;
 using namespace Acts::UnitLiterals;
 using namespace Acts::HashedStringLiteral;
-BOOST_AUTO_TEST_SUITE(PodioTrackConversion)
+BOOST_AUTO_TEST_SUITE(PodioTrackContainerTest)
 
 class NullHelper : public PodioUtil::ConversionHelper {
  public:
@@ -310,7 +311,7 @@ BOOST_AUTO_TEST_CASE(ConvertTrack) {
   }
 }
 
-BOOST_AUTO_TEST_CASE(CopyTracksIncludingDynamicColumns) {
+BOOST_AUTO_TEST_CASE(CopyTracksIncludingDynamicColumnsDifferentBackends) {
   MapHelper helper;
 
   podio::Frame frame;
@@ -368,17 +369,17 @@ BOOST_AUTO_TEST_CASE(CopyTracksIncludingDynamicColumns) {
     BOOST_CHECK_GT(t3.nTrackStates(), 0);
     BOOST_REQUIRE_EQUAL(t.nTrackStates(), t3.nTrackStates());
 
-    // for (auto [tsa, tsb] :
-    // zip(t.trackStatesReversed(), t3.trackStatesReversed())) {
-    // BOOST_CHECK_EQUAL(tsa.predicted(), tsb.predicted());
+    for (auto [tsa, tsb] :
+         zip(t.trackStatesReversed(), t3.trackStatesReversed())) {
+      BOOST_CHECK_EQUAL(tsa.predicted(), tsb.predicted());
 
-    // BOOST_CHECK_EQUAL(
-    // (tsa.template component<std::size_t, "ts_counter"_hash>()),
-    // (tsb.template component<std::size_t, "ts_counter"_hash>()));
+      BOOST_CHECK_EQUAL(
+          (tsa.template component<uint64_t, "ts_counter"_hash>()),
+          (tsb.template component<uint64_t, "ts_counter"_hash>()));
 
-    // BOOST_CHECK_EQUAL((tsa.template component<bool, "ts_odd"_hash>()),
-    // (tsb.template component<bool, "ts_odd"_hash>()));
-    // }
+      BOOST_CHECK_EQUAL((tsa.template component<uint8_t, "ts_odd"_hash>()),
+                        (tsb.template component<uint8_t, "ts_odd"_hash>()));
+    }
 
     BOOST_CHECK_EQUAL(t.template component<uint64_t>("counter"),
                       t3.template component<uint64_t>("counter"));

Tests/UnitTests/Plugins/Podio/PodioTrackStateContainerTest.cpp

@@ -208,8 +208,7 @@ BOOST_AUTO_TEST_CASE(WriteToPodioFrame) {
 
   MapHelper helper;
 
-  // auto tmp_path = std::filesystem::temp_directory_path();
-  auto tmp_path = std::filesystem::current_path();
+  auto tmp_path = std::filesystem::temp_directory_path();
   auto outfile = tmp_path / "trackstates.root";
 
   BoundVector tv1;