From 4751b8c46f9963e1c5966942b2fcb5fe78e804ea Mon Sep 17 00:00:00 2001
From: ralfulrich <ralf.ulrich@kit.edu>
Date: Thu, 16 May 2019 12:44:57 +0200
Subject: [PATCH] fixed bug in SecondaryView::Delete
---
Framework/StackInterface/SecondaryView.h | 39 ++++++++++++++-----
Framework/StackInterface/testSecondaryView.cc | 20 +++-------
2 files changed, 34 insertions(+), 25 deletions(-)
diff --git a/Framework/StackInterface/SecondaryView.h b/Framework/StackInterface/SecondaryView.h
index 1bbbe02cc..fae9d463f 100644
--- a/Framework/StackInterface/SecondaryView.h
+++ b/Framework/StackInterface/SecondaryView.h
@@ -43,14 +43,18 @@ namespace corsika::stack {
All operations possible on a Stack object are also possible on a
SecondaryView object. This means you can add, delete, copy, swap,
iterate, etc.
- */
- /* INTERNAL NOTE FOR DEVELOPERS The secondary particle indices are
- stored in a std::vector fIndices, the index of the primary projectle
- particle is explicitly stored in fProjectileIndex. StackIterator
- indices are refering to those numbers, where
- StackIterator::GetIndex()==0 refers to the fProjectileIndex and
- StackIterator::GetIndex()>0 to fIndices[i+1], see function GetIndexFromIterator.
+ *Further information about implementation (for developers):* All
+ data is stored in the original stack privided at construction
+ time. The secondary particle (view) indices are stored in an
+ extra std::vector of SecondaryView class 'fIndices' referring to
+ the original stack slot indices. The index of the primary
+ projectle particle is also explicitly stored in
+ 'fProjectileIndex'. StackIterator indices
+ 'i = StackIterator::GetIndex()' are referring to those numbers,
+ where 'i==0' refers to the 'fProjectileIndex', and
+ 'StackIterator::GetIndex()>0' to 'fIndices[i-1]', see function
+ GetIndexFromIterator.
*/
template <typename StackDataType, template <typename> typename ParticleInterface>
@@ -125,8 +129,11 @@ namespace corsika::stack {
template <typename... Args>
auto AddSecondary(StackIterator& proj, const Args... v) {
+ // make space on stack
InnerStackType::GetStackData().IncrementSize();
+ // get current number of secondaries on stack
const unsigned int idSec = GetSize();
+ // determine index on (inner) stack where new particle will be located
const unsigned int index = InnerStackType::GetStackData().GetSize() - 1;
fIndices.push_back(index);
// NOTE: "+1" is since "0" is special marker here for PROJECTILE, see
@@ -161,14 +168,26 @@ namespace corsika::stack {
/// @}
/**
- * need overwrite Stack::Delete, since we want to call SecondaryView::DeleteLast
+ * need overwrite Stack::Delete, since we want to call
+ * SecondaryView::DeleteLast
+ *
+ * The particle is deleted on the underlying (internal) stack. The
+ * local references in SecondaryView in fIndices must be fixed,
+ * too. The approach is to a) check if the particle 'p' is at the
+ * very end of the internal stack, b) if not: move it there by
+ * copying the last particle to the current particle location, c)
+ * remove the last particle.
+ *
*/
void Delete(StackIterator p) {
if (IsEmpty()) { /* error */
throw std::runtime_error("Stack, cannot delete entry since size is zero");
}
- if (p.GetIndex() < GetSize() - 1)
- InnerStackType::GetStackData().Copy(GetSize() - 1, p.GetIndex());
+ const int innerSize = InnerStackType::GetSize();
+ const int innerIndex = GetIndexFromIterator(p.GetIndex());
+ if (innerIndex < innerSize - 1)
+ InnerStackType::GetStackData().Copy(innerSize - 1,
+ GetIndexFromIterator(p.GetIndex()));
DeleteLast();
}
diff --git a/Framework/StackInterface/testSecondaryView.cc b/Framework/StackInterface/testSecondaryView.cc
index a306de379..af22f9b17 100644
--- a/Framework/StackInterface/testSecondaryView.cc
+++ b/Framework/StackInterface/testSecondaryView.cc
@@ -137,6 +137,7 @@ TEST_CASE("SecondaryStack", "[stack]") {
SECTION("deletion") {
StackTest stack;
+ stack.AddParticle(std::tuple{-99.});
stack.AddParticle(std::tuple{0.});
{
@@ -149,27 +150,21 @@ TEST_CASE("SecondaryStack", "[stack]") {
proj.AddSecondary(std::tuple{1.});
proj.AddSecondary(std::tuple{2.});
- CHECK(stack.GetSize() == 5);
+ CHECK(stack.GetSize() == 6); // -99, 0, -2, -1, 1, 2
+ CHECK(view.GetSize() == 4); // -2, -1, 1, 2
// now delete all negative entries, i.e. -1 and -2
auto p = view.begin();
while (p != view.end()) {
auto data = p.GetData();
if (data < 0) {
- std::cout << "deleting " << data << ", ";
p.Delete();
} else {
++p;
}
}
-
- // stack should contain 0, 1, 2 now
-
- CHECK(stack.GetSize() == 3);
-
- std::cout << std::endl;
- for (auto& p : stack) { std::cout << p.GetData() << " | "; }
- std::cout << std::endl;
+ CHECK(stack.GetSize() == 4); // -99, 0, 2, 1
+ CHECK(view.GetSize() == 2); // 2, 1
}
// repeat
@@ -190,7 +185,6 @@ TEST_CASE("SecondaryStack", "[stack]") {
while (p != view.end()) {
auto data = p.GetData();
if (data < 0) {
- std::cout << "deleting " << data << ", ";
p.Delete();
} else {
++p;
@@ -201,10 +195,6 @@ TEST_CASE("SecondaryStack", "[stack]") {
// view should contain 1, 2
CHECK(stack.GetSize() == 6);
-
- std::cout << std::endl;
- for (auto& p : stack) { std::cout << p.GetData() << " | "; }
- std::cout << std::endl;
}
}
}
--
GitLab