Skip to content

Commit

Permalink
Merge pull request #269 from veprbl/pr/podio_fix
Browse files Browse the repository at this point in the history 
Fixes for the next PODIO version
  • Loading branch information
@nathanwbrei
nathanwbrei authored Dec 27, 2023
2 parents 3be28c2 + 51af011 commit a14ba26
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 160 deletions.
16 changes: 8 additions & 8 deletions src/examples/PodioExample/PodioExample.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -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");
Expand Down
154 changes: 2 additions & 152 deletions src/programs/unit_tests/PodioTests.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<JEvent>();
event->InsertCollection<ExampleCluster>(std::move(clusters_to_insert), "clusters");
Expand Down Expand Up @@ -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<ExampleClusterCollection>("clusters");
const ExampleClusterCollection* output_coll2 = &f.get<ExampleClusterCollection>("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<ExampleClusterCollection>("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<JEvent>();
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<ExampleCluster>(std::move(clusters_to_insert), "clusters");

// Retrieve via event->GetCollection
const ExampleClusterCollection* retrieved_via_collection = event->GetCollection<ExampleCluster>("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<T>
auto retrieved_via_ptr = event->Get<ExampleCluster>("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<ExampleHitCollection>("hits");
const auto& clusters_out = frame.get<ExampleClusterCollection>("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<T>() 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<ExampleHitCollection>("hits");
const auto& clusters_out = frame.get<ExampleClusterCollection>("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 <typename T, typename = void>
struct MyWrapper {
bool have_podio() {
Expand Down Expand Up @@ -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()));

}

Expand Down

0 comments on commit a14ba26

Please sign in to comment.