diff --git a/src/examples/PodioExample/PodioExample.cc b/src/examples/PodioExample/PodioExample.cc index 6c0c5efea..998328a2a 100644 --- a/src/examples/PodioExample/PodioExample.cc +++ b/src/examples/PodioExample/PodioExample.cc @@ -26,10 +26,10 @@ void create_hits_file() { eventinfos1.push_back(eventinfo1); ExampleHitCollection hits1; - hits1.push_back({22, -1, -1, 0, 100}); - hits1.push_back({49, 1, 1, 0, 15.5}); - hits1.push_back({47, 1, 2, 0, 0.5}); - hits1.push_back({42, 2, 1, 0, 4.0}); + hits1.push_back(ExampleHit({22, -1, -1, 0, 100})); + hits1.push_back(ExampleHit({49, 1, 1, 0, 15.5})); + hits1.push_back(ExampleHit({47, 1, 2, 0, 0.5})); + hits1.push_back(ExampleHit({42, 2, 1, 0, 4.0})); podio::Frame event1; event1.put(std::move(hits1), "hits"); @@ -43,10 +43,10 @@ void create_hits_file() { eventinfos2.push_back(eventinfo2); ExampleHitCollection hits2; - hits2.push_back({42, 5, -5, 5, 7.6}); - hits2.push_back({618, -3, -5, 1, 99.9}); - hits2.push_back({27, -10, 10, 10, 22.2}); - hits2.push_back({28, -9, 11, 10, 7.8}); + hits2.push_back(ExampleHit({42, 5, -5, 5, 7.6})); + hits2.push_back(ExampleHit({618, -3, -5, 1, 99.9})); + hits2.push_back(ExampleHit({27, -10, 10, 10, 22.2})); + hits2.push_back(ExampleHit({28, -9, 11, 10, 7.8})); podio::Frame event2; event2.put(std::move(hits2), "hits"); diff --git a/src/programs/unit_tests/PodioTests.cc b/src/programs/unit_tests/PodioTests.cc index a1de8bde6..3ea642023 100644 --- a/src/programs/unit_tests/PodioTests.cc +++ b/src/programs/unit_tests/PodioTests.cc @@ -11,8 +11,8 @@ namespace podiotests { TEST_CASE("PodioTestsInsertAndRetrieve") { ExampleClusterCollection clusters_to_insert; - clusters_to_insert.push_back({16.0}); - clusters_to_insert.push_back({128.0}); + clusters_to_insert.push_back(ExampleCluster({16.0})); + clusters_to_insert.push_back(ExampleCluster({128.0})); auto event = std::make_shared(); event->InsertCollection(std::move(clusters_to_insert), "clusters"); @@ -46,155 +46,6 @@ TEST_CASE("PodioTestsInsertAndRetrieve") { } -TEST_CASE("PodioTestsShallowCopySemantics") { - - // This one is interesting. The PODIO objects we interact with (e.g. ExampleCluster), are transient lightweight - // wrappers. What's tricky is maintaining a vector of pointers to these things. We can't simply capture a pointer - // to the references returned by the collection iterator because those will all self-destruct. What we can do is - // call the copy constructor. However, we need to make sure that this ctor is doing a shallow copy of the wrapper - // as opposed to a deep copy of the underlying data. - - SECTION("Shallow copy of object wrapper") { - auto a = ExampleCluster(22.2); - auto b = ExampleCluster(a); - REQUIRE(a.id() == b.id()); - REQUIRE(&(a.energy()) == &(b.energy())); // energy() getter returns a const& into the actual data object - } - - SECTION("Transience of collections") { - auto a = MutableExampleCluster(22.2); - ExampleClusterCollection input_coll; - input_coll.push_back(a); - podio::Frame f; - f.put(std::move(input_coll), "clusters"); - - const ExampleClusterCollection* output_coll1 = &f.get("clusters"); - const ExampleClusterCollection* output_coll2 = &f.get("clusters"); - - REQUIRE(output_coll1 == output_coll2); - } - - SECTION("Shallow copy of object upon inserting a collection into a frame") { - - // NOTE: Inserting an object into a collection is NOT necessarily performing a shallow copy. - // At any rate, id and ptr are not preserved. - auto a = MutableExampleCluster(22.2); - // auto a_ptr = &(a.energy()); - // auto a_id= a.id(); - - ExampleClusterCollection input_coll; - input_coll.push_back(a); - ExampleCluster b = input_coll[0]; - // auto b_id = b.id(); - auto b_ptr = &(b.energy()); - - podio::Frame f; - f.put(std::move(input_coll), "clusters"); - auto& c = f.get("clusters"); - // auto c_id = c[0].id(); - auto c_ptr = &(c[0].energy()); - - REQUIRE(b.energy() == 22.2); - REQUIRE(c[0].energy() == 22.2); - REQUIRE(b_ptr == c_ptr); - // REQUIRE(b_id == c_id); // Inserting a Collection into to a Frame changes the id. Is this a problem? - } - - SECTION("In practice") { - - auto event = std::make_shared(); - auto a = MutableExampleCluster(22.2); - // auto a_id = a.id(); - // auto a_ptr = &(a.energy()); - - ExampleClusterCollection clusters_to_insert; - clusters_to_insert.push_back(a); - event->InsertCollection(std::move(clusters_to_insert), "clusters"); - - // Retrieve via event->GetCollection - const ExampleClusterCollection* retrieved_via_collection = event->GetCollection("clusters"); - const ExampleCluster& b = (*retrieved_via_collection)[0]; - auto b_id = b.id(); - auto b_ptr = &(b.energy()); - REQUIRE(b.energy() == 22.2); - - // Retrieve via event->Get - auto retrieved_via_ptr = event->Get("clusters"); - const ExampleCluster* c = retrieved_via_ptr[0]; - auto c_id = c->id(); - auto c_ptr = &(c->energy()); - - REQUIRE(c->energy() == 22.2); - REQUIRE(b_id == c_id); - REQUIRE(b_ptr == c_ptr); - } - - - SECTION("Make sure we aren't accidentally doing a deep copy when we make an association") { - - podio::Frame frame; - MutableExampleHit hit1(1, 11,12,13, 7.7); - ExampleHitCollection hits; - hits.push_back(hit1); - frame.put(std::move(hits), "hits"); - - MutableExampleCluster cluster1(0.0); - cluster1.addHits(hit1); - ExampleClusterCollection clusters; - clusters.push_back(cluster1); - frame.put(std::move(clusters), "clusters"); - - const auto& hits_out = frame.get("hits"); - const auto& clusters_out = frame.get("clusters"); - - REQUIRE(&(clusters_out[0].Hits()[0].energy()) == &(hits_out[0].energy())); - REQUIRE(clusters_out[0].Hits()[0].id() == hits_out[0].id()); - } - - SECTION("Make sure that references established _before_ inserting into frame are preserved") { - // TODO: This needs multifactories before we can test it end-to-end, but we can certainly test - // just the PODIO parts for now - - podio::Frame frame; - MutableExampleHit hit1(1, 11,12,13, 7.7); - MutableExampleHit hit2(1, 10,10,10, 9.2); - - // First make the associations - MutableExampleCluster cluster1(0.0); - cluster1.addHits(hit1); - cluster1.addHits(hit2); - - MutableExampleCluster cluster2(9.0); - cluster2.addHits(hit1); - - // THEN insert into the Collection (This simulates a JANA user calling Set() instead of SetCollection()) - // Note that when inserting into a collection, PODIO rewrites the object ids. - ExampleHitCollection hits; - hits.push_back(hit1); - hits.push_back(hit2); - - ExampleClusterCollection clusters; - clusters.push_back(cluster1); - clusters.push_back(cluster2); - - // Insert collections into the frame last. This will rewrite both sets of ids a second time (!), but stress-tests PODIO the most - frame.put(std::move(hits), "hits"); - frame.put(std::move(clusters), "clusters"); - - const auto& hits_out = frame.get("hits"); - const auto& clusters_out = frame.get("clusters"); - - REQUIRE(&(clusters_out[0].Hits()[0].energy()) == &(hits_out[0].energy())); - REQUIRE(&(clusters_out[0].Hits()[1].energy()) == &(hits_out[1].energy())); - REQUIRE(&(clusters_out[1].Hits()[0].energy()) == &(hits_out[0].energy())); - - REQUIRE(clusters_out[0].Hits()[0].id() == hits_out[0].id()); - REQUIRE(clusters_out[0].Hits()[1].id() == hits_out[1].id()); - REQUIRE(clusters_out[1].Hits()[0].id() == hits_out[0].id()); - } -} - - template struct MyWrapper { bool have_podio() { @@ -291,7 +142,6 @@ TEST_CASE("PODIO 'subset' collections handled correctly (not involving factories const ExampleCluster& c = (*retrieved_subset_clusters)[0]; REQUIRE(c.energy() == 4.0); REQUIRE(c.id() == b.id()); - REQUIRE(&(c.energy()) == &(b.energy())); }