From a4d2de88ae7fba351f73740512d1a28228d1a708 Mon Sep 17 00:00:00 2001 From: RobBuchanan Date: Tue, 23 Jun 2026 15:42:26 +0100 Subject: [PATCH 1/7] structure manipulations and supercellconfigurationnode --- src/classes/structure.cpp | 79 ++++++++++++++++++++++++++++ src/classes/structure.h | 18 +++++++ src/nodes/registry.cpp | 2 + src/nodes/supercellConfiguration.cpp | 70 ++++++++++++++++++++++++ src/nodes/supercellConfiguration.h | 41 +++++++++++++++ 5 files changed, 210 insertions(+) create mode 100644 src/nodes/supercellConfiguration.cpp create mode 100644 src/nodes/supercellConfiguration.h diff --git a/src/classes/structure.cpp b/src/classes/structure.cpp index 3a5d2db1f6..1c0b104627 100644 --- a/src/classes/structure.cpp +++ b/src/classes/structure.cpp @@ -246,6 +246,85 @@ void Structure::createBox(const Matrix3 &axes) box_ = Box::generate(lengths, angles); } +/* + * Manipulations + */ + +// Recursive function for general manipulation +void Structure::recurseLocal(std::set &flags, int indexI, ManipulationFunction action) +{ + auto *strAtI = atoms_[indexI].get(); + + if (flags.find(strAtI) != flags.end()) + return; + + // Set the flag for indexI and get some necessary values + flags.insert(strAtI); + + // Loop over attached atoms, performing minimum image repositioning w.r.t. i, and call the action + for (const auto *b : strAtI->bonds()) + { + auto indexJ = b->partner(strAtI)->index(); + auto &j = atoms_[indexJ]; + if (flags.contains(j.get())) + continue; + + action(j.get(), box_.minimumImage(j->r(), strAtI->r())); + + // Recurse into bound neighbours + recurseLocal(flags, indexJ, action); + } +} +void Structure::recurseLocal(std::set &flags, int indexI, ConstManipulationFunction action) const +{ + auto *strAtI = atoms_[indexI].get(); + + if (flags.find(strAtI) != flags.end()) + return; + + // Set the flag for indexI and get some necessary values + flags.insert(strAtI); + + // Loop over attached atoms, performing minimum image repositioning w.r.t. i, and call the action + for (const auto b : strAtI->bonds()) + { + auto indexJ = b->partner(strAtI)->index(); + auto &j = atoms_[indexJ]; + if (flags.contains(j.get())) + continue; + + action(j.get(), box_.minimumImage(j->r(), strAtI->r())); + + // Recurse into bound neighbours + recurseLocal(flags, indexJ, action); + } +} + +// General manipulation function working on reassembled fragments +std::set Structure::traverseLocal(ManipulationFunction action) +{ + std::set flags; + action(atoms_[0].get(), atoms_[0]->r()); + recurseLocal(flags, 0, action); + return flags; +} +std::set Structure::traverseLocal(ConstManipulationFunction action) const +{ + std::set flags; + action(atoms_[0].get(), atoms_[0]->r()); + recurseLocal(flags, 0, action); + return flags; +} + +// Un-fold molecule so it is not cut by box boundaries +void Structure::unFold() +{ + std::set flagged; + + while (flagged.size() <= nAtoms()) + flagged.merge(traverseLocal([](StructureAtom *j, Vector3 rJ) { j->setR(rJ); })); +} + /* * Serialisation */ diff --git a/src/classes/structure.h b/src/classes/structure.h index 5c9c359ca3..c064c290ef 100644 --- a/src/classes/structure.h +++ b/src/classes/structure.h @@ -116,6 +116,24 @@ class Structure : public Serialisable<> // Create Box definition from axes matrix void createBox(const Matrix3 &axes); + /* + * Manipulations + */ + private: + // Typedef for manipulation functions + using ManipulationFunction = std::function; + using ConstManipulationFunction = std::function; + // Recursive function for general manipulation + void recurseLocal(std::set &flags, int indexI, ManipulationFunction action); + void recurseLocal(std::set &flags, int indexI, ConstManipulationFunction action) const; + // General manipulation function working on reassembled molecule + std::set traverseLocal(ManipulationFunction action); + std::set traverseLocal(ConstManipulationFunction action) const; + + public: + // Un-fold molecule so it is not cut by box boundaries, returning the centre of geometry + void unFold(); + /* * Serialisation */ diff --git a/src/nodes/registry.cpp b/src/nodes/registry.cpp index d2a9ded0e2..695042afa4 100644 --- a/src/nodes/registry.cpp +++ b/src/nodes/registry.cpp @@ -56,6 +56,7 @@ #include "nodes/species.h" #include "nodes/sq.h" #include "nodes/subtract.h" +#include "nodes/supercellConfiguration.h" #include "nodes/vector3Assemble.h" #include "nodes/vector3Decompose.h" #include "nodes/voxelDensity.h" @@ -133,6 +134,7 @@ void NodeRegistry::instantiateNodeProducers() {"SQ", makeDerivedNode()}, {"Species", makeDerivedNode()}, {"Subtract", makeDerivedNode()}, + {"SupercellConfiguration", makeDerivedNode()}, {"Vector3Assemble", makeDerivedNode()}, {"Vector3Decompose", makeDerivedNode()}, {"XRaySQ", makeDerivedNode()}, diff --git a/src/nodes/supercellConfiguration.cpp b/src/nodes/supercellConfiguration.cpp new file mode 100644 index 0000000000..e0b48913b9 --- /dev/null +++ b/src/nodes/supercellConfiguration.cpp @@ -0,0 +1,70 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +// Copyright (c) 2026 Team Dissolve and contributors + +#include "nodes/supercellConfiguration.h" +#include "math/vector3.h" + +SupercellConfigurationNode::SupercellConfigurationNode(Graph *parentGraph) : Node(parentGraph) +{ + // Inputs + addInput("Configuration", "Target configuration", targetConfiguration_); + + // Outputs + addPointerOutput("SupercellConfiguration", "Supercell configuration", supercellConfiguration_); + + // Options + addOption("SupercellRepeat", "Integer coefficients by which unit cell will be repeated along its dimensions", + supercellRepeat_); +} + +/* + * Definition + */ + +std::string_view SupercellConfigurationNode::type() const { return "SupercellConfiguration"; } + +std::string_view SupercellConfigurationNode::summary() const +{ + return "Create a repeated instance (supercell) of a configuration"; +} + +/* + * Processing + */ + +// Run main processing +NodeConstants::ProcessResult SupercellConfigurationNode::process() +{ + supercellConfiguration_.empty(); + + const auto *box = targetConfiguration_->box(); + + // Set up configuration + auto supercellLengths = box->axisLengths(); + supercellLengths.multiply(supercellRepeat_.x, supercellRepeat_.y, supercellRepeat_.z); + supercellConfiguration_.createBoxAndCells(supercellLengths, box->axisAngles(), false); + + // Create images of all molecular unit cell species + for (auto &mol : targetConfiguration_->molecules()) + { + const auto *sp = mol->species(); + + // Loop over cell images + for (auto ix = 0; ix < supercellRepeat_.x; ++ix) + { + for (auto iy = 0; iy < supercellRepeat_.y; ++iy) + { + // Create and translate molecule + for (auto iz = 0; iz < supercellRepeat_.z; ++iz) + supercellConfiguration_.addMolecule(sp)->translate(delta); + } + } + } + + supercellConfiguration_.updateObjectRelationships(); + + message("Created ({}, {}, {}) supercell - {} atoms total.\n", supercellRepeat_.x, supercellRepeat_.y, supercellRepeat_.z, + supercellConfiguration_.nAtoms()); + + return NodeConstants::ProcessResult::Success; +} \ No newline at end of file diff --git a/src/nodes/supercellConfiguration.h b/src/nodes/supercellConfiguration.h new file mode 100644 index 0000000000..1ac1dd9db8 --- /dev/null +++ b/src/nodes/supercellConfiguration.h @@ -0,0 +1,41 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +// Copyright (c) 2026 Team Dissolve and contributors + +#pragma once + +#include "classes/configuration.h" +#include "nodes/node.h" + +// SupercellConfiguration Node +class SupercellConfigurationNode : public Node +{ + public: + SupercellConfigurationNode(Graph *parentGraph); + ~SupercellConfigurationNode() override = default; + + /* + * Definition + */ + + public: + std::string_view type() const override; + std::string_view summary() const override; + + /* + * Data + */ + private: + // Input configuration + Configuration *targetConfiguration_; + // Supercell repeat + Vector3i supercellRepeat_; + // Supercell configuration + Configuration supercellConfiguration_; + + /* + * Processing + */ + protected: + // Perform processing + NodeConstants::ProcessResult process() override; +}; From 53236a7fb701690c7ed9ea21b4ead59b90471567 Mon Sep 17 00:00:00 2001 From: RobBuchanan Date: Tue, 23 Jun 2026 15:55:45 +0100 Subject: [PATCH 2/7] corrections --- src/classes/structure.h | 1 + src/nodes/supercellConfiguration.cpp | 8 ++++---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/classes/structure.h b/src/classes/structure.h index c064c290ef..ca6ca6b5c6 100644 --- a/src/classes/structure.h +++ b/src/classes/structure.h @@ -7,6 +7,7 @@ #include "classes/atom.h" #include "classes/bond.h" #include "classes/box.h" +#include #include // StructureAtom diff --git a/src/nodes/supercellConfiguration.cpp b/src/nodes/supercellConfiguration.cpp index e0b48913b9..1081d6a16e 100644 --- a/src/nodes/supercellConfiguration.cpp +++ b/src/nodes/supercellConfiguration.cpp @@ -37,12 +37,12 @@ NodeConstants::ProcessResult SupercellConfigurationNode::process() { supercellConfiguration_.empty(); - const auto *box = targetConfiguration_->box(); + const auto box = targetConfiguration_->box(); // Set up configuration - auto supercellLengths = box->axisLengths(); + auto supercellLengths = box.axisLengths(); supercellLengths.multiply(supercellRepeat_.x, supercellRepeat_.y, supercellRepeat_.z); - supercellConfiguration_.createBoxAndCells(supercellLengths, box->axisAngles(), false); + supercellConfiguration_.createBoxAndCells(supercellLengths, box.axisAngles(), false); // Create images of all molecular unit cell species for (auto &mol : targetConfiguration_->molecules()) @@ -56,7 +56,7 @@ NodeConstants::ProcessResult SupercellConfigurationNode::process() { // Create and translate molecule for (auto iz = 0; iz < supercellRepeat_.z; ++iz) - supercellConfiguration_.addMolecule(sp)->translate(delta); + supercellConfiguration_.addMolecule(sp)->translate(Vector3(ix, iy, iz)); } } } From 9e99da03be10134d58eb2345edb3422d3822e1b3 Mon Sep 17 00:00:00 2001 From: RobBuchanan Date: Wed, 24 Jun 2026 10:02:13 +0100 Subject: [PATCH 3/7] some pr comments - cosmetic/comments etc --- src/classes/structure.cpp | 2 +- src/classes/structure.h | 2 +- src/nodes/supercellConfiguration.cpp | 9 ++++----- src/nodes/supercellConfiguration.h | 4 +--- 4 files changed, 7 insertions(+), 10 deletions(-) diff --git a/src/classes/structure.cpp b/src/classes/structure.cpp index 1c0b104627..3bc893d8c0 100644 --- a/src/classes/structure.cpp +++ b/src/classes/structure.cpp @@ -316,7 +316,7 @@ std::set Structure::traverseLocal(ConstManipulationFunction act return flags; } -// Un-fold molecule so it is not cut by box boundaries +// Un-fold bound fragments in the structure void Structure::unFold() { std::set flagged; diff --git a/src/classes/structure.h b/src/classes/structure.h index ca6ca6b5c6..b523feed81 100644 --- a/src/classes/structure.h +++ b/src/classes/structure.h @@ -132,7 +132,7 @@ class Structure : public Serialisable<> std::set traverseLocal(ConstManipulationFunction action) const; public: - // Un-fold molecule so it is not cut by box boundaries, returning the centre of geometry + // Un-fold bound fragments in the structure void unFold(); /* diff --git a/src/nodes/supercellConfiguration.cpp b/src/nodes/supercellConfiguration.cpp index 1081d6a16e..4b70ad5861 100644 --- a/src/nodes/supercellConfiguration.cpp +++ b/src/nodes/supercellConfiguration.cpp @@ -7,14 +7,13 @@ SupercellConfigurationNode::SupercellConfigurationNode(Graph *parentGraph) : Node(parentGraph) { // Inputs - addInput("Configuration", "Target configuration", targetConfiguration_); + addInput("Configuration", "Target configuration", targetConfiguration_); // Outputs - addPointerOutput("SupercellConfiguration", "Supercell configuration", supercellConfiguration_); + addPointerOutput("SupercellConfiguration", "Supercell configuration", supercellConfiguration_); // Options - addOption("SupercellRepeat", "Integer coefficients by which unit cell will be repeated along its dimensions", - supercellRepeat_); + addOption("SupercellRepeat", "Integer coefficients by which unit cell will be repeated along its axes", supercellRepeat_); } /* @@ -32,7 +31,7 @@ std::string_view SupercellConfigurationNode::summary() const * Processing */ -// Run main processing +// Perform processing NodeConstants::ProcessResult SupercellConfigurationNode::process() { supercellConfiguration_.empty(); diff --git a/src/nodes/supercellConfiguration.h b/src/nodes/supercellConfiguration.h index 1ac1dd9db8..02b66a4113 100644 --- a/src/nodes/supercellConfiguration.h +++ b/src/nodes/supercellConfiguration.h @@ -6,7 +6,6 @@ #include "classes/configuration.h" #include "nodes/node.h" -// SupercellConfiguration Node class SupercellConfigurationNode : public Node { public: @@ -16,7 +15,6 @@ class SupercellConfigurationNode : public Node /* * Definition */ - public: std::string_view type() const override; std::string_view summary() const override; @@ -26,7 +24,7 @@ class SupercellConfigurationNode : public Node */ private: // Input configuration - Configuration *targetConfiguration_; + Configuration *targetConfiguration_{nullptr}; // Supercell repeat Vector3i supercellRepeat_; // Supercell configuration From b0e7299e9e9f09908be75c7e20264f8e6ea09325 Mon Sep 17 00:00:00 2001 From: RobBuchanan Date: Wed, 24 Jun 2026 11:59:18 +0100 Subject: [PATCH 4/7] pr comments and test --- src/classes/structure.cpp | 72 ++++++++++++++-------------- src/classes/structure.h | 10 ++-- src/nodes/supercellConfiguration.cpp | 2 +- tests/classes/structure.cpp | 22 +++++++++ tests/data/xyz/ch4_folded.xyz | 17 +++++++ 5 files changed, 81 insertions(+), 42 deletions(-) create mode 100644 tests/data/xyz/ch4_folded.xyz diff --git a/src/classes/structure.cpp b/src/classes/structure.cpp index 3bc893d8c0..94d9f5e52f 100644 --- a/src/classes/structure.cpp +++ b/src/classes/structure.cpp @@ -251,78 +251,78 @@ void Structure::createBox(const Matrix3 &axes) */ // Recursive function for general manipulation -void Structure::recurseLocal(std::set &flags, int indexI, ManipulationFunction action) +void Structure::recurseLocal(std::set &fragmentAtoms, StructureAtom *i, ManipulationFunction action) { - auto *strAtI = atoms_[indexI].get(); - - if (flags.find(strAtI) != flags.end()) + if (fragmentAtoms.contains(i)) return; // Set the flag for indexI and get some necessary values - flags.insert(strAtI); + fragmentAtoms.insert(i); // Loop over attached atoms, performing minimum image repositioning w.r.t. i, and call the action - for (const auto *b : strAtI->bonds()) + for (const auto *b : i->bonds()) { - auto indexJ = b->partner(strAtI)->index(); - auto &j = atoms_[indexJ]; - if (flags.contains(j.get())) + auto j = b->partner(i); + if (fragmentAtoms.contains(j)) continue; - action(j.get(), box_.minimumImage(j->r(), strAtI->r())); + action(j, box_.minimumImage(j->r(), i->r())); // Recurse into bound neighbours - recurseLocal(flags, indexJ, action); + recurseLocal(fragmentAtoms, j, action); } } -void Structure::recurseLocal(std::set &flags, int indexI, ConstManipulationFunction action) const +void Structure::recurseLocal(std::set &fragmentAtoms, StructureAtom *i, ConstManipulationFunction action) const { - auto *strAtI = atoms_[indexI].get(); - - if (flags.find(strAtI) != flags.end()) + if (fragmentAtoms.contains(i)) return; // Set the flag for indexI and get some necessary values - flags.insert(strAtI); + fragmentAtoms.insert(i); // Loop over attached atoms, performing minimum image repositioning w.r.t. i, and call the action - for (const auto b : strAtI->bonds()) + for (const auto b : i->bonds()) { - auto indexJ = b->partner(strAtI)->index(); - auto &j = atoms_[indexJ]; - if (flags.contains(j.get())) + auto j = b->partner(i); + if (fragmentAtoms.contains(j)) continue; - action(j.get(), box_.minimumImage(j->r(), strAtI->r())); + action(j, box_.minimumImage(j->r(), i->r())); // Recurse into bound neighbours - recurseLocal(flags, indexJ, action); + recurseLocal(fragmentAtoms, j, action); } } -// General manipulation function working on reassembled fragments -std::set Structure::traverseLocal(ManipulationFunction action) +// Return atoms in the same fragment as the specified atom, unfolding the fragment at the same time +std::set Structure::getUnfoldedFragment(StructureAtom *containing, ManipulationFunction action) { - std::set flags; - action(atoms_[0].get(), atoms_[0]->r()); - recurseLocal(flags, 0, action); - return flags; + std::set fragmentAtoms; + action(containing, containing->r()); + recurseLocal(fragmentAtoms, containing, action); + return fragmentAtoms; } -std::set Structure::traverseLocal(ConstManipulationFunction action) const +std::set Structure::getUnfoldedFragment(StructureAtom *containing, ConstManipulationFunction action) const { - std::set flags; - action(atoms_[0].get(), atoms_[0]->r()); - recurseLocal(flags, 0, action); - return flags; + std::set fragmentAtoms; + action(containing, containing->r()); + recurseLocal(fragmentAtoms, containing, action); + return fragmentAtoms; } // Un-fold bound fragments in the structure void Structure::unFold() { - std::set flagged; + std::set fragmentAtoms; - while (flagged.size() <= nAtoms()) - flagged.merge(traverseLocal([](StructureAtom *j, Vector3 rJ) { j->setR(rJ); })); + while (fragmentAtoms.size() < nAtoms()) + { + auto atomIt = std::find_if(atoms_.begin(), atoms_.end(), [&fragmentAtoms](const auto &atom) { return !fragmentAtoms.contains(atom.get()); }); + if (atomIt == atoms_.end()) + break; + + fragmentAtoms.merge(getUnfoldedFragment(atomIt->get(), [](StructureAtom *j, Vector3 rJ) { j->setR(rJ); })); + } } /* diff --git a/src/classes/structure.h b/src/classes/structure.h index b523feed81..79f084ec61 100644 --- a/src/classes/structure.h +++ b/src/classes/structure.h @@ -125,11 +125,11 @@ class Structure : public Serialisable<> using ManipulationFunction = std::function; using ConstManipulationFunction = std::function; // Recursive function for general manipulation - void recurseLocal(std::set &flags, int indexI, ManipulationFunction action); - void recurseLocal(std::set &flags, int indexI, ConstManipulationFunction action) const; - // General manipulation function working on reassembled molecule - std::set traverseLocal(ManipulationFunction action); - std::set traverseLocal(ConstManipulationFunction action) const; + void recurseLocal(std::set &fragmentAtoms, StructureAtom *i, ManipulationFunction action); + void recurseLocal(std::set &fragmentAtoms, StructureAtom *i, ConstManipulationFunction action) const; + // Return atoms in the same fragment as the specified atom, unfolding the fragment at the same time + std::set getUnfoldedFragment(StructureAtom *containing, ManipulationFunction action); + std::set getUnfoldedFragment(StructureAtom *containing, ConstManipulationFunction action) const; public: // Un-fold bound fragments in the structure diff --git a/src/nodes/supercellConfiguration.cpp b/src/nodes/supercellConfiguration.cpp index 4b70ad5861..82d9f38cb1 100644 --- a/src/nodes/supercellConfiguration.cpp +++ b/src/nodes/supercellConfiguration.cpp @@ -55,7 +55,7 @@ NodeConstants::ProcessResult SupercellConfigurationNode::process() { // Create and translate molecule for (auto iz = 0; iz < supercellRepeat_.z; ++iz) - supercellConfiguration_.addMolecule(sp)->translate(Vector3(ix, iy, iz)); + supercellConfiguration_.addMolecule(sp)->translate(box.axes() * Vector3(ix, iy, iz)); } } } diff --git a/tests/classes/structure.cpp b/tests/classes/structure.cpp index f45717d007..c33727b577 100644 --- a/tests/classes/structure.cpp +++ b/tests/classes/structure.cpp @@ -3,6 +3,8 @@ #include "classes/structure.h" #include "nodes/calculateBonding.h" +#include "tests/graphData.h" +#include #include namespace UnitTest @@ -68,4 +70,24 @@ TEST(StructureTest, Molecule2) EXPECT_EQ(s.nAtoms(), 0); } +TEST(StructureTest, Unfold) +{ + TestGraph graph; + auto importXYZNode = graph.createNode("ImportXYZStructure"); + ASSERT_TRUE(importXYZNode); + ASSERT_TRUE(importXYZNode->setOption("FilePath", std::string("xyz/ch4_folded.xyz"))); + ASSERT_EQ(importXYZNode->run(), NodeConstants::ProcessResult::Success); + auto structure = importXYZNode->getOutputValue("Structure"); + structure.unFold(); + + // After unfolding, the distances between C and H should all be unity + for (const auto &bond : structure.bonds()) + { + auto iR = bond->i()->r(); + auto jR = bond->j()->r(); + auto distance = abs((iR - jR).magnitude()); + ASSERT_NEAR(distance, 1, 10e-9); + } +} + } // namespace UnitTest diff --git a/tests/data/xyz/ch4_folded.xyz b/tests/data/xyz/ch4_folded.xyz new file mode 100644 index 0000000000..e59f7b7701 --- /dev/null +++ b/tests/data/xyz/ch4_folded.xyz @@ -0,0 +1,17 @@ +15 +Unnamed001 +C 5.000000 0.100000 5.000000 0.000000 +H 5.000000 1.100000 5.000000 0.000000 +H 5.000000 9.766193 4.057359 0.000000 +H 5.816351 9.766193 5.471321 0.000000 +H 4.183649 9.766193 5.471321 0.000000 +C 0.100000 5.000000 5.000000 0.000000 +H 1.100000 5.000000 5.000000 0.000000 +H 9.766193 5.000000 5.942641 0.000000 +H 9.766193 5.816351 4.528679 0.000000 +H 9.766193 4.183649 4.528679 0.000000 +C 5.000000 5.000000 0.100000 0.000000 +H 5.000000 5.000000 1.100000 0.000000 +H 5.000000 5.942641 9.766193 0.000000 +H 5.816351 4.528679 9.766193 0.000000 +H 4.183649 4.528679 9.766193 0.000000 From e3cfaf54e6cdf4455cf18b98019432e13144e66a Mon Sep 17 00:00:00 2001 From: RobBuchanan Date: Wed, 24 Jun 2026 13:28:29 +0100 Subject: [PATCH 5/7] clang format --- src/classes/structure.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/classes/structure.cpp b/src/classes/structure.cpp index 94d9f5e52f..7b8b23db64 100644 --- a/src/classes/structure.cpp +++ b/src/classes/structure.cpp @@ -317,7 +317,8 @@ void Structure::unFold() while (fragmentAtoms.size() < nAtoms()) { - auto atomIt = std::find_if(atoms_.begin(), atoms_.end(), [&fragmentAtoms](const auto &atom) { return !fragmentAtoms.contains(atom.get()); }); + auto atomIt = std::find_if(atoms_.begin(), atoms_.end(), + [&fragmentAtoms](const auto &atom) { return !fragmentAtoms.contains(atom.get()); }); if (atomIt == atoms_.end()) break; From 3dcbc5dc5703d13e2dfafbfc2e7a92f3b31206bb Mon Sep 17 00:00:00 2001 From: RobBuchanan Date: Wed, 24 Jun 2026 14:49:00 +0100 Subject: [PATCH 6/7] test passing --- src/classes/structure.cpp | 2 -- src/nodes/supercellConfiguration.cpp | 2 +- tests/classes/structure.cpp | 15 +++++++++++++-- 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/classes/structure.cpp b/src/classes/structure.cpp index 7b8b23db64..564c639cfb 100644 --- a/src/classes/structure.cpp +++ b/src/classes/structure.cpp @@ -256,7 +256,6 @@ void Structure::recurseLocal(std::set &fragmentAtoms, Structure if (fragmentAtoms.contains(i)) return; - // Set the flag for indexI and get some necessary values fragmentAtoms.insert(i); // Loop over attached atoms, performing minimum image repositioning w.r.t. i, and call the action @@ -277,7 +276,6 @@ void Structure::recurseLocal(std::set &fragmentAtoms, Structure if (fragmentAtoms.contains(i)) return; - // Set the flag for indexI and get some necessary values fragmentAtoms.insert(i); // Loop over attached atoms, performing minimum image repositioning w.r.t. i, and call the action diff --git a/src/nodes/supercellConfiguration.cpp b/src/nodes/supercellConfiguration.cpp index 82d9f38cb1..213f7d4b0e 100644 --- a/src/nodes/supercellConfiguration.cpp +++ b/src/nodes/supercellConfiguration.cpp @@ -36,7 +36,7 @@ NodeConstants::ProcessResult SupercellConfigurationNode::process() { supercellConfiguration_.empty(); - const auto box = targetConfiguration_->box(); + const auto &box = targetConfiguration_->box(); // Set up configuration auto supercellLengths = box.axisLengths(); diff --git a/tests/classes/structure.cpp b/tests/classes/structure.cpp index c33727b577..3b1c9771b8 100644 --- a/tests/classes/structure.cpp +++ b/tests/classes/structure.cpp @@ -73,11 +73,22 @@ TEST(StructureTest, Molecule2) TEST(StructureTest, Unfold) { TestGraph graph; + + // Import XYZ node auto importXYZNode = graph.createNode("ImportXYZStructure"); ASSERT_TRUE(importXYZNode); ASSERT_TRUE(importXYZNode->setOption("FilePath", std::string("xyz/ch4_folded.xyz"))); - ASSERT_EQ(importXYZNode->run(), NodeConstants::ProcessResult::Success); - auto structure = importXYZNode->getOutputValue("Structure"); + + // Calculate bonding node + auto calculateBondingNode = graph.createNode("CalculateBonding"); + ASSERT_TRUE(calculateBondingNode); + + // Connect graph + ASSERT_TRUE(graph.addEdge({"ImportXYZStructure", "Structure", "CalculateBonding", "Structure"})); + + // Run + ASSERT_EQ(calculateBondingNode->run(), NodeConstants::ProcessResult::Success); + auto structure = calculateBondingNode->getOutputValue("Structure"); structure.unFold(); // After unfolding, the distances between C and H should all be unity From d46deedd0a4a1ecd8d4646dfc83990cd9b5d80fd Mon Sep 17 00:00:00 2001 From: RobBuchanan Date: Wed, 24 Jun 2026 14:53:11 +0100 Subject: [PATCH 7/7] refactor loop --- src/classes/structure.cpp | 8 +++----- tests/classes/structure.cpp | 2 ++ 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/classes/structure.cpp b/src/classes/structure.cpp index 564c639cfb..2c20f0d8a2 100644 --- a/src/classes/structure.cpp +++ b/src/classes/structure.cpp @@ -313,14 +313,12 @@ void Structure::unFold() { std::set fragmentAtoms; - while (fragmentAtoms.size() < nAtoms()) + for (auto &atom : atoms()) { - auto atomIt = std::find_if(atoms_.begin(), atoms_.end(), - [&fragmentAtoms](const auto &atom) { return !fragmentAtoms.contains(atom.get()); }); - if (atomIt == atoms_.end()) + if (fragmentAtoms.contains(atom.get())) break; - fragmentAtoms.merge(getUnfoldedFragment(atomIt->get(), [](StructureAtom *j, Vector3 rJ) { j->setR(rJ); })); + fragmentAtoms.merge(getUnfoldedFragment(atom.get(), [](StructureAtom *j, Vector3 rJ) { j->setR(rJ); })); } } diff --git a/tests/classes/structure.cpp b/tests/classes/structure.cpp index 3b1c9771b8..e07c5032b5 100644 --- a/tests/classes/structure.cpp +++ b/tests/classes/structure.cpp @@ -88,7 +88,9 @@ TEST(StructureTest, Unfold) // Run ASSERT_EQ(calculateBondingNode->run(), NodeConstants::ProcessResult::Success); + auto structure = calculateBondingNode->getOutputValue("Structure"); + ASSERT_TRUE(structure.bonds().size() != 0); structure.unFold(); // After unfolding, the distances between C and H should all be unity