From a4e684f03905cd0d3017e25c403490224e293307 Mon Sep 17 00:00:00 2001 From: Juan Miguel Carceller <22276694+jmcarcell@users.noreply.github.com> Date: Tue, 4 Jun 2024 13:12:45 +0200 Subject: [PATCH] Add a parameter `cloneRelations` for `clone` to be able not to clone the collections (#609) * Fix the equal comparison for relations of cloned objects * Fix pre-commit * Revert previous commits This reverts commit e8b962aaf8a91eaed8126a4ee8c96f5a77eb0711. * Add a parameter to clone with empty relations * Add tests adding to the relations of cloned objects * Change from emptyRelations to cloneRelations * Remove std::cout in the test case * Add short documentation for cloning --------- Co-authored-by: jmcarcell Co-authored-by: tmadlener --- doc/examples.md | 10 + doc/figures/object_clone.svg | 194 ++++++++++++++++++ python/templates/MutableObject.cc.jinja2 | 2 +- python/templates/macros/declarations.jinja2 | 3 +- .../templates/macros/implementations.jinja2 | 35 +++- tests/unittests/unittest.cpp | 28 +++ 6 files changed, 262 insertions(+), 10 deletions(-) create mode 100644 doc/figures/object_clone.svg diff --git a/doc/examples.md b/doc/examples.md index 637abc5cf..770089fae 100644 --- a/doc/examples.md +++ b/doc/examples.md @@ -86,6 +86,16 @@ and via direct object access: } ``` +### Cloneing objects + +In order to clone objects it is necessary to use the `clone` method which +returns a `Mutable` object again. `clone` takes a parameter `cloneRelations` +which is defaulted to `true`. By default the relation information of the +original object is simply copied over, changing this to `false` only copies the +data but none of the relations. + +![](figures/object_clone.svg) + ### Support for Notebook-Pattern The `notebook pattern` uses the assumption that it is better to create a small diff --git a/doc/figures/object_clone.svg b/doc/figures/object_clone.svg new file mode 100644 index 000000000..14d04ae4b --- /dev/null +++ b/doc/figures/object_clone.svg @@ -0,0 +1,194 @@ + + + + + + + + + + + + + + + + + + + + + + + + + original + + clone(false) + + clone() + + + + + + diff --git a/python/templates/MutableObject.cc.jinja2 b/python/templates/MutableObject.cc.jinja2 index eaec2f716..d94b4f32c 100644 --- a/python/templates/MutableObject.cc.jinja2 +++ b/python/templates/MutableObject.cc.jinja2 @@ -16,7 +16,7 @@ {{ utils.namespace_open(class.namespace) }} -{{ macros.constructors_destructors(class.bare_type, Members, prefix='Mutable') }} +{{ macros.constructors_destructors(class.bare_type, Members, multi_relations=OneToManyRelations + VectorMembers, prefix='Mutable') }} {{ macros.member_getters(class, Members, use_get_syntax, prefix='Mutable') }} {{ macros.single_relation_getters(class, OneToOneRelations, use_get_syntax, prefix='Mutable') }} diff --git a/python/templates/macros/declarations.jinja2 b/python/templates/macros/declarations.jinja2 index 41a2eb288..5ef5b8707 100644 --- a/python/templates/macros/declarations.jinja2 +++ b/python/templates/macros/declarations.jinja2 @@ -74,7 +74,8 @@ {{ full_type }}& operator=({{ full_type }} other); /// create a mutable deep-copy of the object with identical relations - Mutable{{ type }} clone() const; + /// if cloneRelations=false, the relations are not cloned and will be empty + Mutable{{ type }} clone(bool cloneRelations=true) const; /// destructor ~{{ full_type }}() = default; diff --git a/python/templates/macros/implementations.jinja2 b/python/templates/macros/implementations.jinja2 index 58861c439..5dbf6d92a 100644 --- a/python/templates/macros/implementations.jinja2 +++ b/python/templates/macros/implementations.jinja2 @@ -18,22 +18,41 @@ return *this; } -Mutable{{ type }} {{ full_type }}::clone() const { +Mutable{{ type }} {{ full_type }}::clone(bool cloneRelations) const { {% if prefix %} + if (!cloneRelations) { + auto tmp = new {{ type }}Obj(podio::ObjectID{}, m_obj->data); +{% for relation in multi_relations %} + tmp->m_{{ relation.name }} = new std::vector<{{ relation.full_type }}>(); + tmp->data.{{ relation.name }}_begin = 0; + tmp->data.{{ relation.name }}_end = 0; +{% endfor %} + return Mutable{{ type }}(podio::utils::MaybeSharedPtr(tmp, podio::utils::MarkOwned)); + } return Mutable{{ type }}(podio::utils::MaybeSharedPtr(new {{ type }}Obj(*m_obj), podio::utils::MarkOwned)); {% else %} auto tmp = new {{ type }}Obj(podio::ObjectID{}, m_obj->data); {% for relation in multi_relations %} tmp->m_{{ relation.name }} = new std::vector<{{ relation.full_type }}>(); - // If the current object has been read from a file, then the object may only have a slice of the relation vector - // so this slice has to be copied in case we want to modify it - tmp->m_{{ relation.name }}->reserve(m_obj->m_{{ relation.name }}->size()); - for (size_t i = m_obj->data.{{ relation.name }}_begin; i < m_obj->data.{{ relation.name }}_end; i++) { - tmp->m_{{ relation.name }}->emplace_back((*m_obj->m_{{ relation.name }})[i]); +{% endfor %} + if (cloneRelations) { +{% for relation in multi_relations %} + // If the current object has been read from a file, then the object may only have a slice of the relation vector + // so this slice has to be copied in case we want to modify it + tmp->m_{{ relation.name }}->reserve(m_obj->m_{{ relation.name }}->size()); + for (size_t i = m_obj->data.{{ relation.name }}_begin; i < m_obj->data.{{ relation.name }}_end; i++) { + tmp->m_{{ relation.name }}->emplace_back((*m_obj->m_{{ relation.name }})[i]); + } + tmp->data.{{ relation.name }}_begin = 0; + tmp->data.{{ relation.name }}_end = tmp->m_{{ relation.name }}->size(); +{% endfor %} } - tmp->data.{{ relation.name }}_begin = 0; - tmp->data.{{ relation.name }}_end = tmp->m_{{ relation.name }}->size(); + else { +{% for relation in multi_relations %} + tmp->data.{{ relation.name }}_begin = 0; + tmp->data.{{ relation.name }}_end = 0; {% endfor %} + } return Mutable{{ type }}(podio::utils::MaybeSharedPtr(tmp, podio::utils::MarkOwned)); {% endif %} } diff --git a/tests/unittests/unittest.cpp b/tests/unittests/unittest.cpp index 934577bb3..0d42ff6fe 100644 --- a/tests/unittests/unittest.cpp +++ b/tests/unittests/unittest.cpp @@ -1385,3 +1385,31 @@ TEST_CASE("Relations after cloning with SIO", "[relations][basics]") { } #endif + +TEST_CASE("Clone empty relations", "[relations][basics]") { + auto coll = ExampleClusterCollection(); + coll.create(); + coll.create(); + coll[0].addHits(ExampleHit()); + coll[0].addHits(ExampleHit()); + auto newColl = ExampleClusterCollection(); + newColl.push_back(coll.at(0).clone(false)); + newColl.push_back(coll.at(1).clone(false)); + REQUIRE(newColl[0].Hits().empty()); + REQUIRE(newColl[1].Hits().empty()); + newColl[0].addHits(ExampleHit()); + REQUIRE(newColl[0].Hits().size() == 1); + newColl[0].addHits(ExampleHit()); + REQUIRE(newColl[0].Hits().size() == 2); + + auto immCluster = ExampleCluster(coll.at(0)); + auto immCluster2 = ExampleCluster(coll.at(1)); + auto clonedImmCluster = immCluster.clone(false); + auto clonedImmCluster2 = immCluster2.clone(false); + REQUIRE(clonedImmCluster.Hits().empty()); + REQUIRE(clonedImmCluster2.Hits().empty()); + clonedImmCluster.addHits(ExampleHit()); + REQUIRE(clonedImmCluster.Hits().size() == 1); + clonedImmCluster.addHits(ExampleHit()); + REQUIRE(clonedImmCluster.Hits().size() == 2); +}