From 9303bb2bd8bdea606eccf8fbd891a59c51b8f86d Mon Sep 17 00:00:00 2001 From: RobBuchanan Date: Mon, 22 Jun 2026 15:02:24 +0100 Subject: [PATCH 1/8] refactor detect molecules process --- src/classes/fragment.h | 25 ++ src/classes/structure.cpp | 9 + src/classes/structure.h | 5 + src/nodes/detectMolecules.cpp | 300 +++++++++++++++++++++++ src/nodes/detectMolecules.h | 75 ++++++ src/nodes/registry.cpp | 2 + tests/nodes/cif.cpp | 431 ++++++++++++++++++++-------------- 7 files changed, 665 insertions(+), 182 deletions(-) create mode 100644 src/nodes/detectMolecules.cpp create mode 100644 src/nodes/detectMolecules.h diff --git a/src/classes/fragment.h b/src/classes/fragment.h index c623cca757..0d309532d2 100644 --- a/src/classes/fragment.h +++ b/src/classes/fragment.h @@ -28,6 +28,24 @@ template class Fragment getIndicesRecursive(atoms, indices, j->index(), exclusions); } } + static void getIndicesRecursive(const std::vector> &atoms, std::vector &indices, int index, + const std::vector &exclusions) + { + // Loop over bonds on indexed atom + indices.emplace_back(index); + const auto i = atoms.at(index).get(); + for (const auto *bond : i->bonds()) + { + // Is this either of the excluded bonds? + if (std::ranges::find(exclusions, bond) != exclusions.end()) + continue; + + // Get the partner atom in the bond and select it (if it is not selected already) + auto j = bond->partner(i); + if (std::find(indices.begin(), indices.end(), j->index()) == indices.end()) + getIndicesRecursive(atoms, indices, j->index(), exclusions); + } + } public: // Return the fragment (vector of indices) containing the specified atom @@ -38,4 +56,11 @@ template class Fragment getIndicesRecursive(atoms, indices, startIndex, exclusions); return indices; } + static std::vector get(const std::vector> &atoms, int startIndex, + const std::vector &exclusions = {}) + { + std::vector indices; + getIndicesRecursive(atoms, indices, startIndex, exclusions); + return indices; + } }; diff --git a/src/classes/structure.cpp b/src/classes/structure.cpp index 2c20f0d8a2..261afcbd10 100644 --- a/src/classes/structure.cpp +++ b/src/classes/structure.cpp @@ -14,15 +14,20 @@ Structure &Structure::operator=(const Structure &source) { clear(); + // Copy atoms for (auto &atom : source.atoms_) { auto &i = atoms_.emplace_back(std::make_unique()); i->copy(*atom); } + // Copy bonds for (auto &bond : source.bonds_) addBond(bond->i()->index(), bond->j()->index()); + // Copy instances + instances_ = source.instances_; + // Copy source box createBox(source.box_.axisLengths(), source.box_.axisAngles(), source.box_.type() == Box::BoxType::None); @@ -119,6 +124,10 @@ const StructureAtom *Structure::atom(int i) const { return atoms_[i].get(); } const std::vector> &Structure::atoms() const { return atoms_; } std::vector> &Structure::atoms() { return atoms_; } +// Return molecular species coordinates +const std::vector> &Structure::instances() const { return instances_; } +std::vector> &Structure::instances() { return instances_; } + /* * Connectivity */ diff --git a/src/classes/structure.h b/src/classes/structure.h index 79f084ec61..6886bc203c 100644 --- a/src/classes/structure.h +++ b/src/classes/structure.h @@ -52,6 +52,8 @@ class Structure : public Serialisable<> private: // Atoms in the structure std::vector> atoms_; + // Positional instances of the root structure + std::vector> instances_; private: // Renumber atoms so they are sequential in the vector @@ -74,6 +76,9 @@ class Structure : public Serialisable<> // Return atoms const std::vector> &atoms() const; std::vector> &atoms(); + // Return positional instances of the root structure + const std::vector> &instances() const; + std::vector> &instances(); /* * Connectivity diff --git a/src/nodes/detectMolecules.cpp b/src/nodes/detectMolecules.cpp new file mode 100644 index 0000000000..4e856fa6b7 --- /dev/null +++ b/src/nodes/detectMolecules.cpp @@ -0,0 +1,300 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +// Copyright (c) 2026 Team Dissolve and contributors + +#include "nodes/detectMolecules.h" +#include "classes/empiricalFormula.h" +#include "classes/fragment.h" +#include "classes/molecule.h" +#include "classes/species.h" +#include +#include + +DetectMoleculesNode::DetectMoleculesNode(Graph *parentGraph) : Node(parentGraph) +{ + // Inputs + addInput("Structure", "Input structure", inputStructure_); +} + +std::string_view DetectMoleculesNode::type() const { return "DetectMolecules"; } + +std::string_view DetectMoleculesNode::summary() const { return "Detect molecular species within a structure"; } + +std::vector> DetectMoleculesNode::findMolecularFragments(const Structure &structure) const +{ + std::vector> fragments; + + auto fragment = [structure](int i) { return Fragment>::get(structure.atoms(), i); }; + + for (int i = 0; i < structure.nAtoms(); i++) + fragments.emplace_back(fragment(i)); + + return fragments; +} + +// Run main processing +NodeConstants::ProcessResult DetectMoleculesNode::process() +{ + detectedStructures_.clear(); + + // Return all discovered molecular fragment index vectors + auto allFragmentIndices = findMolecularFragments(inputStructure_); + + // Try selecting within the species from the first atom - if this captures all atoms we have a bound framework... + if (allFragmentIndices[0].size() == inputStructure_.nAtoms()) + return error( + "Can't create molecular definitions since this unit cell appears to be a continuous framework/network. Consider " + "adjusting the bonding options in order to generate molecular fragments.\n"); + + std::set atomMask; + + for (int i = 0; i < inputStructure_.nAtoms(); i++) + { + const auto structureAtom = inputStructure_.atom(i); + if (atomMask.contains(structureAtom)) + continue; + + auto fragmentIndices = allFragmentIndices.at(i); + + auto largeFragment = fragmentIndices.size() * 2 > inputStructure_.nAtoms(); + + // Find instances of this fragment. + for (auto fragAtomIndex : fragmentIndices) + { + const auto fragmentAtom = inputStructure_.atom(fragAtomIndex); + + // Create a provisional structure for the detected fragment + Structure detectedMolStructure; + detectedMolStructure.createBox(inputStructure_.box().axes()); + std::vector> instances; + + // Skip NETA definition creation for large fragments + if (largeFragment) + { + if (atomMask.contains(fragmentAtom)) + continue; + + auto &instance = instances.emplace_back(); + for (auto fragAtomIndex : fragmentIndices) + { + auto fragmentAtom = inputStructure_.atom(fragAtomIndex); + instance.push_back(fragmentAtom->r()); + atomMask.insert(fragmentAtom); + + // Remove fragments with this size + std::erase_if(allFragmentIndices, [&fragmentIndices](const auto &fragment) + { return fragment.size() == fragmentIndices.size(); }); + } + } + else + { + /* + * Best NETA definition + */ + + // Set up the return value and bind its contents + NETADefinition bestNETA; + std::vector rootAtoms; + + // Maintain a set of atoms matched by any NETA description we generate + std::set alreadyMatched; + + // Skip this atom? + if (alreadyMatched.find(fragmentAtom) != alreadyMatched.end()) + continue; + + // Create a NETA definition with this atom as the root + NETADefinition neta; + neta.create(static_cast(fragmentAtom), std::nullopt, + Flags(NETADefinition::NETACreationFlags::ExplicitHydrogens, + NETADefinition::NETACreationFlags::IncludeRootElement)); + + // Apply this match over the whole species + std::vector currentRootAtoms; + for (auto fragAtomIndex : fragmentIndices) + { + const auto fragmentAtom = inputStructure_.atom(fragAtomIndex); + if (neta.matches(fragmentAtom)) + { + currentRootAtoms.push_back(fragmentAtom); + alreadyMatched.insert(fragmentAtom); + } + } + + // Is this a better description? + auto better = false; + if (rootAtoms.empty() || currentRootAtoms.size() < rootAtoms.size()) + better = true; + else if (currentRootAtoms.size() == rootAtoms.size()) + { + // Replace the current match if there are more bonds on the current atom. + if (fragmentAtom->nBonds() > rootAtoms.front()->nBonds()) + better = true; + } + + if (better) + { + bestNETA = neta; + rootAtoms = currentRootAtoms; + } + + /* + * Get instances + */ + + // Iterate over all structural atoms, matching their unit cell atoms by NETA + std::vector> matchedUnitCellAtomSets; + for (const auto &structureAtom : inputStructure_.atoms()) + { + if (atomMask.contains(structureAtom.get())) + continue; + + auto matchedPath = neta.matchedPath(structureAtom.get()).set(); + if (!matchedPath.empty()) + { + auto set = matchedUnitCellAtomSets.emplace_back(matchedPath); + for (const auto &matchedAtom : set) + atomMask.insert(static_cast(matchedAtom)); + } + } + + // Loop over matched unit cell atoms, retrieving instances + for (const auto &matchedUnitCellAtoms : matchedUnitCellAtomSets) + { + if (matchedUnitCellAtoms.empty()) + continue; + + auto &instance = instances.emplace_back(); + for (const auto &matchedAtom : matchedUnitCellAtoms) + instance.push_back(matchedAtom->r()); + } + } + + // Detect structures that have instances + if (!instances.empty()) + detectedStructures_ + .emplace_back(copyStructureAtomsAndBonds(inputStructure_, detectedMolStructure, fragmentIndices)) + .instances() = instances; + } + } + + // Unfold all detected structures + for (auto &structure : detectedStructures_) + structure.unFold(); + + message("Detected {} distinct fragment structures:\n\n", detectedStructures_.size()); + message(" ID N Species Formula\n"); + auto count = 1; + for (const auto &structure : detectedStructures_) + message(" {:3d} {:4d} {}\n", count++, structure.instances().size(), + EmpiricalFormula::formula(structure.atoms(), [](const auto &i) { return i->Z(); })); + message(""); + + return NodeConstants::ProcessResult::Success; +} + +/* + * Helpers + */ + +// Copy atom and bond information from one structure to another +Structure &DetectMoleculesNode::copyStructureAtomsAndBonds(const Structure &source, Structure &target, + const std::vector fragmentIndices) +{ + // Copy fragment atoms, forming a map of the original indices to the new atom in the structure + std::map originalIndexMap; + for (auto fragAtomIndex : fragmentIndices) + { + const auto fragmentAtom = source.atom(fragAtomIndex); + originalIndexMap[fragAtomIndex] = target.addAtom(fragmentAtom->Z(), fragmentAtom->r(), fragmentAtom->q()); + std::cout << std::format("New atom added to structure: {} {}\n", fragAtomIndex, Elements::symbol(fragmentAtom->Z())); + } + + // Copy bond information - since our fragment is by definition a bound fragment, we copy all bonds on each atom + for (auto fragAtomIndex : fragmentIndices) + { + const auto fragmentAtom = source.atom(fragAtomIndex); + for (auto bond : fragmentAtom->bonds()) + { + // Add a bond between the new atoms in the detected structure (as long as it doesn't already exist) + if (!target.hasBond(originalIndexMap[bond->i()->index()], originalIndexMap[bond->j()->index()])) + target.addBond(originalIndexMap[bond->i()->index()], originalIndexMap[bond->j()->index()]); + } + } + + return target; +} + +// Recursively check NETA description matches between the supplied atoms +std::map +DetectMoleculesNode::matchAtom(const StructureAtom *referenceAtom, const StructureAtom *instanceAtom, + const std::map &refNETA, + const std::map &map) +{ + // If the reference atom NETA doesn't match the instance atom we cannot proceed + if (!refNETA.at(referenceAtom).matches(instanceAtom)) + return {}; + + // Check the map to see if we have already associated the reference atom to an instance atom, or if the instance atom + // is already associated to a different reference atom. + for (auto &&[mappedRefAtom, mappedInstanceAtom] : map) + { + // Found it - double-check to ensure that the current association matches our instance atom. If it does we can return + // the map as it currently stands. If not we return an empty map to indicate failure. + if (mappedRefAtom == referenceAtom) + { + if (mappedInstanceAtom == instanceAtom) + { + return map; + } + else + { + return {}; + } + } + else if (mappedInstanceAtom == instanceAtom) + { + return {}; + } + } + + // Copy the current map, associate our initial pair of atoms and try to extend it + auto newMap = map; + newMap[referenceAtom] = instanceAtom; + + // Cycle over bonds on the reference atom and find + for (const auto &referenceBond : referenceAtom->bonds()) + { + // Get the reference bond partner + auto *referenceBondPartner = referenceBond->partner(referenceAtom); + + // Try to find a match over bonds on the instance atom + std::map bondResult; + for (const auto &instanceBond : instanceAtom->bonds()) + { + // Get the instance bond partner + auto *instanceBondPartner = instanceBond->partner(instanceAtom); + + // Recurse + bondResult = matchAtom(referenceBondPartner, instanceBondPartner, refNETA, newMap); + if (!bondResult.empty()) + break; + } + + // If we found a suitable match recursing into the bond, store the result into newMap and continue to the next bond. + // If we didn't find a good match, we return now. + if (bondResult.empty()) + return {}; + + newMap = bondResult; + } + + // If we get to here then we succeeded, so return the new map + return newMap; +} + +/* + * Getters + */ + +// Output structures +const std::vector &DetectMoleculesNode::detectedStructures() const { return detectedStructures_; } \ No newline at end of file diff --git a/src/nodes/detectMolecules.h b/src/nodes/detectMolecules.h new file mode 100644 index 0000000000..3bcd80c341 --- /dev/null +++ b/src/nodes/detectMolecules.h @@ -0,0 +1,75 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +// Copyright (c) 2026 Team Dissolve and contributors + +#pragma once + +#include "classes/localMolecule.h" +#include "classes/molecule.h" +#include "classes/species.h" +#include "classes/structure.h" +#include "data/elements.h" +#include "math/vector3.h" +#include "nodes/node.h" +#include +#include +#include + +class Species; + +// DetectMolecules Node +class DetectMoleculesNode : public Node +{ + public: + DetectMoleculesNode(Graph *parentGraph); + ~DetectMoleculesNode() override = default; + + public: + std::string_view type() const override; + std::string_view summary() const override; + + /* + * Definition + */ + private: + // Input structure + Structure inputStructure_; + // Output structures + std::vector detectedStructures_; + // Best NETA definitions for structure atoms + std::map bestNetaDefinitions_; + + /* + * Helpers + */ + private: + // Copy atom and bond information from one structure to another + static Structure ©StructureAtomsAndBonds(const Structure &source, Structure &target, + const std::vector fragmentIndices); + // Find molecular fragments + std::vector> findMolecularFragments(const Structure &structure) const; + // Determine the best NETA definition for the supplied species + std::tuple> bestNETADefinition(const Structure &structure); + // Get instances for the supplied species from the cleaned unit cell + std::vector> getInstances(const Structure &referenceStructure, std::vector &atomMask, + const NETADefinition &neta, + const std::vector &referenceRootAtoms); + // Recursively check NETA description matches between the supplied atoms + std::map + matchAtom(const StructureAtom *referenceAtom, const StructureAtom *instanceAtom, + const std::map &refNETA, + const std::map &map); + + /* + * Processing + */ + private: + // Run main processing + NodeConstants::ProcessResult process() override; + + /* + * Getters + */ + public: + // Output structures + const std::vector &detectedStructures() const; +}; diff --git a/src/nodes/registry.cpp b/src/nodes/registry.cpp index 695042afa4..1dda84d876 100644 --- a/src/nodes/registry.cpp +++ b/src/nodes/registry.cpp @@ -14,6 +14,7 @@ #include "nodes/configuration.h" #include "nodes/dAngle.h" #include "nodes/derivative.h" +#include "nodes/detectMolecules.h" #include "nodes/dotProduct.h" #include "nodes/edge.h" #include "nodes/energy.h" @@ -92,6 +93,7 @@ void NodeRegistry::instantiateNodeProducers() {"ImportCIFStructure", makeDerivedNode()}, {"DAngle", makeDerivedNode()}, {"Derivative", makeDerivedNode()}, + {"DetectMolecules", makeDerivedNode()}, {"DotProduct", makeDerivedNode()}, {"Energy", makeDerivedNode()}, {"EPSR", makeDerivedNode()}, diff --git a/tests/nodes/cif.cpp b/tests/nodes/cif.cpp index 49516c7486..e9a32f688b 100644 --- a/tests/nodes/cif.cpp +++ b/tests/nodes/cif.cpp @@ -2,7 +2,9 @@ // Copyright (c) 2026 Team Dissolve and contributors #include "classes/empiricalFormula.h" +#include "nodes/calculateBonding.h" #include "nodes/cif/importCIFStructure.h" +#include "nodes/detectMolecules.h" #include "tests/graphData.h" #include "tests/testData.h" #include @@ -16,34 +18,9 @@ class CIFNodeTest : public ::testing::Test CIFNodeTest() = default; ~CIFNodeTest() = default; - protected: - TestGraph testGraph_; - const std::string delimiter_{".cif"}; - const std::string path_{"cif/"}; - public: // Molecular species information using MolecularSpeciesInfo = std::tuple; - // Create CIF graph - void createGraph(std::string filename) - { - auto name = cifNameFromFile(filename); - EXPECT_TRUE(testGraph_.appendNode("ImportCIFStructure", name)); - testGraph_.fetchHead()->setOption("FilePath", path_ + filename); - } - // Determine CIF node name from filename - std::string cifNameFromFile(std::string filename) - { - auto name = filename.substr(0, filename.find(delimiter_)); - return name; - } - // Retrieve CIF context by filename - ImportCIFStructureNode *getContextByFileName(std::string filename) - { - auto name = cifNameFromFile(filename); - auto node = testGraph_.findNode(name); - return static_cast(node); - } // Test Box definition void testBox(const Configuration *cfg, const Vector3 &lengths, const Vector3 &angles, int nAtoms) { @@ -57,12 +34,13 @@ class CIFNodeTest : public ::testing::Test EXPECT_NEAR(cfg->box().axisAngles().z, angles.z, 1.0e-6); } // Test molecular species information provided - void testMolecularSpecies(const CIFMolecularSpecies &molSp, const MolecularSpeciesInfo &info) + void testDetectedMolecularStructure(const Structure &structure, const MolecularSpeciesInfo &info) { - EXPECT_EQ(molSp.species()->name(), std::get<0>(info)); - EXPECT_EQ(molSp.instances().size(), std::get<1>(info)); - EXPECT_EQ(molSp.species()->nAtoms(), std::get<2>(info)); + // EXPECT_EQ(structure.name(), std::get<0>(info)); + EXPECT_EQ(structure.instances().size(), std::get<1>(info)); + EXPECT_EQ(structure.nAtoms(), std::get<2>(info)); } + /* // Check instance consistency with reference coordinates void testInstanceConsistency(const CIFMolecularSpecies &molSp, const Species &referenceCoordinates) { @@ -87,241 +65,330 @@ class CIFNodeTest : public ::testing::Test } } } + */ }; TEST_F(CIFNodeTest, Parse) { + TestGraph testGraph; + // Test files with expected number of structure atoms std::vector> cifs = {{"1557470.cif", 86}, {"1557599.cif", 56}, {"7705246.cif", 364}, {"9000004.cif", 6}, {"9000095.cif", 30}, {"9000418.cif", 64}}; for (auto &[cif, nStructureAtoms] : cifs) { - createGraph(cif); - auto node = testGraph_.findNode(cifNameFromFile(cif)); - ASSERT_EQ(node->run(), NodeConstants::ProcessResult::Success); - const auto structure = node->getOutputValue("Structure"); + ASSERT_TRUE(testGraph.appendNode("ImportCIFStructure", cif)); + testGraph.fetchHead()->setOption("FilePath", "cif/" + cif); + ASSERT_EQ(testGraph.fetchHead()->run(), NodeConstants::ProcessResult::Success); + const auto structure = testGraph.fetchHead()->getOutputValue("Structure"); ASSERT_EQ(structure.atoms().size(), nStructureAtoms); } } -/* -TEST_F(CIFNodeTest, NaCl) +TEST_F(CIFNodeTest, NaClContinuous) { + TestGraph testGraph; + // Load the CIF file - auto cif = "NaCl-1000041.cif"; - createGraph(cif); - auto loaderNode = testGraph_.findNode(cifNameFromFile(cif)); - ASSERT_EQ(loaderNode->run(), NodeConstants::ProcessResult::Success); + auto cif = std::string("NaCl-1000041.cif"); - auto cifContext = getContextByFileName(cif); - ASSERT_TRUE(cifContext); - EXPECT_TRUE(cifContext->generate()); + EXPECT_TRUE(testGraph.appendNode("ImportCIFStructure")); + testGraph.fetchHead()->setOption("FilePath", "cif/" + cif); + EXPECT_TRUE(testGraph.appendNode("CalculateBonding")); + ASSERT_TRUE(testGraph.appendNode("DetectMolecules")); + testGraph.addEdge({"ImportCIFStructure", "Structure", "CalculateBonding", "Structure"}); + testGraph.addEdge({"CalculateBonding", "Structure", "DetectMolecules", "Structure"}); - // Check basic info - auto molecularSpeciesNode = testGraph_.findNode(cifNameFromFile(cif) + "//MolecularSpecies"); - ASSERT_EQ(molecularSpeciesNode->run(), NodeConstants::ProcessResult::Success); + // Basic info + ASSERT_EQ(testGraph.findNode("CalculateBonding")->run(), NodeConstants::ProcessResult::Success); + EXPECT_EQ(testGraph.findNode("ImportCIFStructure")->findOption("SpaceGroupID")->get(), + SpaceGroups::SpaceGroup_225); - EXPECT_EQ(cifContext->spaceGroup(), SpaceGroups::SpaceGroup_225); - constexpr double A = 5.62; - testBox(molecularSpeciesNode->getOutputValue("SupercellConfiguration"), {A, A, A}, {90, 90, 90}, 8); + // constexpr double A = 5.62; - // Calculating bonding is the default, but this gives a continuous framework... - EXPECT_EQ(molecularSpeciesNode->getOutputValue>("DetectedMolecularSpecies").size(), 0); + // We should find a continuous framework after rebonding and the detect molecules node should fail accordingly + ASSERT_EQ(testGraph.findNode("DetectMolecules")->run(), NodeConstants::ProcessResult::Failed); +} - // Get molecular species - auto bondingNode = testGraph_.findNode(cifNameFromFile(cif) + "//BondingOptions"); - bondingNode->setOption("UseCIFBondingDefinitions", true); - testGraph_.setUpdateRequired(); - ASSERT_EQ(molecularSpeciesNode->run(), NodeConstants::ProcessResult::Success); +TEST_F(CIFNodeTest, NaClMolecules) +{ + TestGraph testGraph; - auto molecularSpecies = molecularSpeciesNode->getOutputValue>("DetectedMolecularSpecies"); + // Load the CIF file + auto cif = std::string("NaCl-1000041.cif"); - EXPECT_EQ(molecularSpecies.size(), 2); - testMolecularSpecies(molecularSpecies.at(0), {"Na", 4, 1}); + EXPECT_TRUE(testGraph.appendNode("ImportCIFStructure")); + testGraph.fetchHead()->setOption("FilePath", "cif/" + cif); + ASSERT_TRUE(testGraph.appendNode("DetectMolecules")); + testGraph.addEdge({"ImportCIFStructure", "Structure", "DetectMolecules", "Structure"}); + + auto detectMoleculesNode = static_cast(testGraph.findNode("DetectMolecules")); + ASSERT_EQ(detectMoleculesNode->run(), NodeConstants::ProcessResult::Success); + + // Basic info + EXPECT_EQ(testGraph.findNode("ImportCIFStructure")->findOption("SpaceGroupID")->get(), + SpaceGroups::SpaceGroup_225); + constexpr double A = 5.62; + + EXPECT_EQ(detectMoleculesNode->detectedStructures().size(), 2); + testDetectedMolecularStructure(detectMoleculesNode->detectedStructures().at(0), {"Na", 4, 1}); std::vector R = {{0.0, 0.0, 0.0}, {0.0, A / 2, A / 2}, {A / 2, 0.0, A / 2}, {A / 2, A / 2, 0.0}}; - for (auto &&[instance, r2] : zip(molecularSpecies.at(0).instances(), R)) - DissolveSystemTest::checkVec3(instance.localAtoms()[0].r(), r2); - testMolecularSpecies(molecularSpecies.at(1), {"Cl", 4, 1}); - for (auto &&[instance, r2] : zip(molecularSpecies.at(1).instances(), R)) - DissolveSystemTest::checkVec3(instance.localAtoms()[0].r(), (r2 - A / 2).abs()); + for (auto &&[instance, r2] : zip(detectMoleculesNode->detectedStructures().at(0).instances(), R)) + DissolveSystemTest::checkVec3(instance[0], r2); + testDetectedMolecularStructure(detectMoleculesNode->detectedStructures().at(1), {"Cl", 4, 1}); + for (auto &&[instance, r2] : zip(detectMoleculesNode->detectedStructures().at(1).instances(), R)) + DissolveSystemTest::checkVec3(instance[0], (r2 - A / 2).abs()); // 2x2x2 supercell - molecularSpeciesNode->setOption("SupercellRepeat", {2, 2, 2}); - testGraph_.dissolveGraph()->setUpdateRequired(); - ASSERT_EQ(molecularSpeciesNode->run(), NodeConstants::ProcessResult::Success); - testBox(molecularSpeciesNode->getOutputValue("SupercellConfiguration"), {A * 2, A * 2, A * 2}, + /* TODO: Handle supercell configurations + detectMoleculesNode->setOption("SupercellRepeat", {2, 2, 2}); + testGraph.dissolveGraph()->setUpdateRequired(); + ASSERT_EQ(detectMoleculesNode->run(), NodeConstants::ProcessResult::Success); + testBox(detectMoleculesNode->getOutputValue("SupercellConfiguration"), {A * 2, A * 2, A * 2}, {90, 90, 90}, 8 * 8); + */ } TEST_F(CIFNodeTest, NaClO3) { + TestGraph testGraph; + // Load the CIF file - auto cif = "NaClO3-1010057.cif"; - createGraph(cif); - ASSERT_EQ(testGraph_.findNode(cifNameFromFile(cif))->run(), NodeConstants::ProcessResult::Success); + auto cif = std::string("NaClO3-1010057.cif"); + + EXPECT_TRUE(testGraph.appendNode("ImportCIFStructure")); + testGraph.fetchHead()->setOption("FilePath", "cif/" + cif); + ASSERT_TRUE(testGraph.appendNode("DetectMolecules")); + testGraph.addEdge({"ImportCIFStructure", "Structure", "DetectMolecules", "Structure"}); - auto cifContext = getContextByFileName(cif); - ASSERT_TRUE(cifContext); - EXPECT_TRUE(cifContext->generate()); + ASSERT_EQ(testGraph.findNode("ImportCIFStructure")->run(), NodeConstants::ProcessResult::Success); // Check basic info - auto molecularSpeciesNode = testGraph_.findNode(cifNameFromFile(cif) + "//MolecularSpecies"); - ASSERT_EQ(molecularSpeciesNode->run(), NodeConstants::ProcessResult::Success); + auto detectMoleculesNode = static_cast(testGraph.findNode("DetectMolecules")); - EXPECT_EQ(cifContext->spaceGroup(), SpaceGroups::SpaceGroup_198); + EXPECT_EQ(testGraph.findNode("ImportCIFStructure")->findOption("SpaceGroupID")->get(), + SpaceGroups::SpaceGroup_198); constexpr double A = 6.55; - testBox(molecularSpeciesNode->getOutputValue("SupercellConfiguration"), {A, A, A}, {90, 90, 90}, 20); + // TODO: Handle supercell configurations + // testBox(detectMoleculesNode->getOutputValue("SupercellConfiguration"), {A, A, A}, {90, 90, 90}, 20); - // Turn off automatic bond calculation - there are no bonding defs in the CIF, so we expect species for each atomic + // No bonding defs in the CIF, so we expect species for each atomic // component (4 Na, 4 Cl, and 12 O) - auto bondingNode = testGraph_.findNode(cifNameFromFile(cif) + "//BondingOptions"); - bondingNode->setOption("UseCIFBondingDefinitions", true); - testGraph_.setUpdateRequired(); - ASSERT_EQ(molecularSpeciesNode->run(), NodeConstants::ProcessResult::Success); + ASSERT_EQ(detectMoleculesNode->run(), NodeConstants::ProcessResult::Success); - auto cifMolsA = molecularSpeciesNode->getOutputValue>("DetectedMolecularSpecies"); - ASSERT_EQ(cifMolsA.size(), 3); - testMolecularSpecies(cifMolsA.at(0), {"Na", 4, 1}); - testMolecularSpecies(cifMolsA.at(1), {"Cl", 4, 1}); - testMolecularSpecies(cifMolsA.at(2), {"O", 12, 1}); + auto detectedMoleculeStructureA = detectMoleculesNode->detectedStructures(); + testDetectedMolecularStructure(detectedMoleculeStructureA.at(0), {"Na", 4, 1}); + testDetectedMolecularStructure(detectedMoleculeStructureA.at(1), {"Cl", 4, 1}); + testDetectedMolecularStructure(detectedMoleculeStructureA.at(2), {"O", 12, 1}); // Calculate bonding ourselves to get the correct species - bondingNode->setOption("UseCIFBondingDefinitions", false); - testGraph_.setUpdateRequired(); - ASSERT_EQ(molecularSpeciesNode->run(), NodeConstants::ProcessResult::Success); - auto cifMolsB = molecularSpeciesNode->getOutputValue>("DetectedMolecularSpecies"); - ASSERT_EQ(cifMolsB.size(), 2); - testMolecularSpecies(cifMolsB.at(0), {"Na", 4, 1}); - testMolecularSpecies(cifMolsB.at(1), {"ClO3", 4, 4}); + EXPECT_TRUE(testGraph.appendNode("CalculateBonding")); + testGraph.removeEdge({"ImportCIFStructure", "Structure", "DetectMolecules", "Structure"}); + testGraph.addEdge({"ImportCIFStructure", "Structure", "CalculateBonding", "Structure"}); + testGraph.addEdge({"CalculateBonding", "Structure", "DetectMolecules", "Structure"}); + + testGraph.setUpdateRequired(); + ASSERT_EQ(detectMoleculesNode->run(), NodeConstants::ProcessResult::Success); + auto detectedMoleculeStructureB = detectMoleculesNode->detectedStructures(); + ASSERT_EQ(detectedMoleculeStructureB.size(), 2); + testDetectedMolecularStructure(detectedMoleculeStructureB.at(0), {"Na", 4, 1}); + testDetectedMolecularStructure(detectedMoleculeStructureB.at(1), {"ClO3", 4, 4}); } TEST_F(CIFNodeTest, CuBTC) { + TestGraph testGraph; + // Load the CIF file - auto cif = "CuBTC-7108574.cif"; - createGraph(cif); - ASSERT_EQ(testGraph_.findNode(cifNameFromFile(cif))->run(), NodeConstants::ProcessResult::Success); + auto cif = std::string("CuBTC-7108574.cif"); - auto cifContext = getContextByFileName(cif); - ASSERT_TRUE(cifContext); - EXPECT_TRUE(cifContext->generate()); + EXPECT_TRUE(testGraph.appendNode("ImportCIFStructure")); + testGraph.fetchHead()->setOption("FilePath", "cif/" + cif); + ASSERT_TRUE(testGraph.appendNode("CalculateBonding")); + testGraph.fetchHead()->setOption("Clear", true); + ASSERT_TRUE(testGraph.appendNode("DetectMolecules")); + testGraph.addEdge({"ImportCIFStructure", "Structure", "CalculateBonding", "Structure"}); + testGraph.addEdge({"CalculateBonding", "Structure", "DetectMolecules", "Structure"}); + + ASSERT_EQ(testGraph.findNode("ImportCIFStructure")->run(), NodeConstants::ProcessResult::Success); // Check basic info - auto molecularSpeciesNode = testGraph_.findNode(cifNameFromFile(cif) + "//MolecularSpecies"); - ASSERT_EQ(molecularSpeciesNode->run(), NodeConstants::ProcessResult::Success); + auto detectMoleculesNode = static_cast(testGraph.findNode("DetectMolecules")); + ASSERT_EQ(detectMoleculesNode->run(), NodeConstants::ProcessResult::Success); - EXPECT_EQ(cifContext->spaceGroup(), SpaceGroups::SpaceGroup_225); + EXPECT_EQ(testGraph.findNode("ImportCIFStructure")->findOption("SpaceGroupID")->get(), + SpaceGroups::SpaceGroup_225); constexpr auto A = 26.3336; - testBox(molecularSpeciesNode->getOutputValue("SupercellConfiguration"), {A, A, A}, {90, 90, 90}, 672); + // testBox(detectMoleculesNode->getOutputValue("SupercellConfiguration"), {A, A, A}, {90, 90, 90}, 672); + // Check basic formula (which includes bound water oxygens - with no H - at this point) and using O group + /* TODO: Handle supercell configurations // 16 basic formula units per unit cell constexpr auto N = 16; - - // Check basic formula (which includes bound water oxygens - with no H - at this point) and using O group EmpiricalFormula::EmpiricalFormulaMap cellFormulaH = { {Elements::Cu, 3 * N}, {Elements::C, 18 * N}, {Elements::H, 6 * N}, {Elements::O, 15 * N}}; - EXPECT_EQ( - EmpiricalFormula::formula(molecularSpeciesNode->getOutputValue("SupercellConfiguration")->atoms(), - [](const auto &i) { return i.speciesAtom()->Z(); }), - EmpiricalFormula::formula(cellFormulaH)); - auto cifMolsA = molecularSpeciesNode->getOutputValue>("DetectedMolecularSpecies"); - EXPECT_EQ(cifMolsA.size(), 2); - + EXPECT_EQ(EmpiricalFormula::formula(detectMoleculesNode->getOutputValue("SupercellConfiguration")->atoms(), + [](const auto &i) { return i.speciesAtom()->Z(); }), + EmpiricalFormula::formula(cellFormulaH)); + */ + auto detectedMoleculeStructureA = detectMoleculesNode->detectedStructures(); + EXPECT_EQ(detectedMoleculeStructureA.size(), 2); +} +/* +TEST_F(CIFNodeTest, CuBTCActiveAssemblies) +{ + TestGraph testGraph; // Change active assemblies to get amine-substituted structure + EmpiricalFormula::EmpiricalFormulaMap cellFormulaNH2 = cellFormulaH; cellFormulaNH2[Elements::N] = 6 * N; cellFormulaNH2[Elements::H] *= 2; - EXPECT_TRUE(testGraph_.appendNode("SetCIFAtomGroupActivity", cifNameFromFile(cif) + "//AtomGroupA1")); - testGraph_.fetchHead()->setOption("Assembly", std::string("A")); - testGraph_.fetchHead()->setOption("AtomGroup", std::string("1")); - testGraph_.fetchHead()->setOption("SetActive", false); - EXPECT_TRUE(testGraph_.appendNode("SetCIFAtomGroupActivity", cifNameFromFile(cif) + "//AtomGroupB2")); - testGraph_.fetchHead()->setOption("Assembly", std::string("B")); - testGraph_.fetchHead()->setOption("AtomGroup", std::string("2")); - testGraph_.fetchHead()->setOption("SetActive", true); - EXPECT_TRUE(testGraph_.appendNode("SetCIFAtomGroupActivity", cifNameFromFile(cif) + "//AtomGroupC2")); - testGraph_.fetchHead()->setOption("Assembly", std::string("C")); - testGraph_.fetchHead()->setOption("AtomGroup", std::string("2")); - testGraph_.fetchHead()->setOption("SetActive", true); - testGraph_.removeEdge( - {cifNameFromFile(cif) + "//StructureCleanup", "CIFContext", std::string(molecularSpeciesNode->name()), "CIFContext"}); - testGraph_.addEdge( + EXPECT_TRUE(testGraph.appendNode("SetCIFAtomGroupActivity", cifNameFromFile(cif) + "//AtomGroupA1")); + testGraph.fetchHead()->setOption("Assembly", std::string("A")); + testGraph.fetchHead()->setOption("AtomGroup", std::string("1")); + testGraph.fetchHead()->setOption("SetActive", false); + EXPECT_TRUE(testGraph.appendNode("SetCIFAtomGroupActivity", cifNameFromFile(cif) + "//AtomGroupB2")); + testGraph.fetchHead()->setOption("Assembly", std::string("B")); + testGraph.fetchHead()->setOption("AtomGroup", std::string("2")); + testGraph.fetchHead()->setOption("SetActive", true); + EXPECT_TRUE(testGraph.appendNode("SetCIFAtomGroupActivity", cifNameFromFile(cif) + "//AtomGroupC2")); + testGraph.fetchHead()->setOption("Assembly", std::string("C")); + testGraph.fetchHead()->setOption("AtomGroup", std::string("2")); + testGraph.fetchHead()->setOption("SetActive", true); + testGraph.removeEdge( + {cifNameFromFile(cif) + "//StructureCleanup", "CIFContext", std::string(detectMoleculesNode->name()), "CIFContext"}); + testGraph.addEdge( {cifNameFromFile(cif) + "//StructureCleanup", "CIFContext", cifNameFromFile(cif) + "//AtomGroupA1", "CIFContext"}); - testGraph_.addEdge( + testGraph.addEdge( {cifNameFromFile(cif) + "//AtomGroupA1", "CIFContext", cifNameFromFile(cif) + "//AtomGroupB2", "CIFContext"}); - testGraph_.addEdge( + testGraph.addEdge( {cifNameFromFile(cif) + "//AtomGroupB2", "CIFContext", cifNameFromFile(cif) + "//AtomGroupC2", "CIFContext"}); - testGraph_.addEdge( - {cifNameFromFile(cif) + "//AtomGroupC2", "CIFContext", std::string(molecularSpeciesNode->name()), "CIFContext"}); - testGraph_.setUpdateRequired(); - ASSERT_EQ(molecularSpeciesNode->run(), NodeConstants::ProcessResult::Success); - EXPECT_EQ( - EmpiricalFormula::formula(molecularSpeciesNode->getOutputValue("SupercellConfiguration")->atoms(), - [](const auto &i) { return i.speciesAtom()->Z(); }), - EmpiricalFormula::formula(cellFormulaNH2)); + testGraph.addEdge( + {cifNameFromFile(cif) + "//AtomGroupC2", "CIFContext", std::string(detectMoleculesNode->name()), "CIFContext"}); + testGraph.setUpdateRequired(); + ASSERT_EQ(detectMoleculesNode->run(), NodeConstants::ProcessResult::Success); + EXPECT_EQ(EmpiricalFormula::formula(detectMoleculesNode->getOutputValue("SupercellConfiguration")->atoms(), + [](const auto &i) { return i.speciesAtom()->Z(); }), + EmpiricalFormula::formula(cellFormulaNH2)); // Remove those free oxygens so we just have a framework - auto removeAtomicsNode = testGraph_.findNode(cifNameFromFile(cif) + "//RemoveAtomic"); + auto removeAtomicsNode = testGraph.findNode(cifNameFromFile(cif) + "//RemoveAtomic"); removeAtomicsNode->setOption("RemoveAtomics", true); - testGraph_.setUpdateRequired(); - ASSERT_EQ(molecularSpeciesNode->run(), NodeConstants::ProcessResult::Success); - auto cifMolsB = molecularSpeciesNode->getOutputValue>("DetectedMolecularSpecies"); - EXPECT_EQ(cifMolsB.size(), 0); + testGraph.setUpdateRequired(); + ASSERT_EQ(detectMoleculesNode->run(), NodeConstants::ProcessResult::Success); + auto detectedMoleculeStructureB = detectMoleculesNode->detectedStructures(); + EXPECT_EQ(detectedMoleculeStructureB.size(), 0); + } +*/ -TEST_F(CIFNodeTest, MoleculeOrdering) +TEST_F(CIFNodeTest, MoleculeOrderingSimple) { - const auto cifFiles = {"molecule-test-simple-ordered.cif", "molecule-test-simple-unordered.cif", - "molecule-test-simple-unordered-rotated.cif"}; - for (auto cifFile : cifFiles) - { - // Load the CIF file - createGraph(cifFile); - ASSERT_EQ(testGraph_.findNode(cifNameFromFile(cifFile))->run(), NodeConstants::ProcessResult::Success); + TestGraph testGraph; - auto cifContext = getContextByFileName(cifFile); - ASSERT_TRUE(cifContext); - EXPECT_TRUE(cifContext->generate()); + auto cif = std::string("molecule-test-simple-ordered.cif"); - auto molecularSpeciesNode = testGraph_.findNode(cifNameFromFile(cifFile) + "//MolecularSpecies"); - ASSERT_EQ(molecularSpeciesNode->run(), NodeConstants::ProcessResult::Success); + EXPECT_TRUE(testGraph.appendNode("ImportCIFStructure")); + testGraph.fetchHead()->setOption("FilePath", "cif/" + cif); + ASSERT_TRUE(testGraph.appendNode("CalculateBonding")); + ASSERT_TRUE(testGraph.appendNode("DetectMolecules", "DetectMolecules")); + testGraph.addEdge({"ImportCIFStructure", "Structure", "CalculateBonding", "Structure"}); + testGraph.addEdge({"CalculateBonding", "Structure", "DetectMolecules", "Structure"}); - auto molecularSpecies = - molecularSpeciesNode->getOutputValue>("DetectedMolecularSpecies"); - EXPECT_EQ(molecularSpecies.size(), 1); + auto detectMoleculesNode = static_cast(testGraph.findNode("DetectMolecules")); + ASSERT_EQ(detectMoleculesNode->run(), NodeConstants::ProcessResult::Success); - auto &cifMolecule = molecularSpecies.front(); - EmpiricalFormula::EmpiricalFormulaMap moleculeFormula = { - {Elements::Cl, 1}, {Elements::O, 1}, {Elements::C, 1}, {Elements::H, 3}}; - testMolecularSpecies(cifMolecule, {EmpiricalFormula::formula(moleculeFormula), 6, 6}); + auto detectedMoleculeStructures = detectMoleculesNode->detectedStructures(); + EXPECT_EQ(detectedMoleculeStructures.size(), 1); - auto &unitCellSpecies = static_cast(molecularSpeciesNode)->cleanedUnitCellSpecies(); - testInstanceConsistency(cifMolecule, unitCellSpecies); - } + auto &molStructure = detectedMoleculeStructures.front(); + EmpiricalFormula::EmpiricalFormulaMap moleculeFormula = { + {Elements::Cl, 1}, {Elements::O, 1}, {Elements::C, 1}, {Elements::H, 3}}; + testDetectedMolecularStructure(molStructure, {EmpiricalFormula::formula(moleculeFormula), 6, 6}); + + // auto &unitCellSpecies = static_cast(detectMoleculesNode)->cleanedUnitCellSpecies(); + // testInstanceConsistency(cifMolecule, unitCellSpecies); +} + +TEST_F(CIFNodeTest, MoleculeOrderingSimpleUnordered) +{ + TestGraph testGraph; + + auto cif = std::string("molecule-test-simple-ordered.cif"); + + EXPECT_TRUE(testGraph.appendNode("ImportCIFStructure")); + testGraph.fetchHead()->setOption("FilePath", "cif/" + cif); + ASSERT_TRUE(testGraph.appendNode("CalculateBonding")); + ASSERT_TRUE(testGraph.appendNode("DetectMolecules", "DetectMolecules")); + testGraph.addEdge({"ImportCIFStructure", "Structure", "CalculateBonding", "Structure"}); + testGraph.addEdge({"CalculateBonding", "Structure", "DetectMolecules", "Structure"}); + + auto detectMoleculesNode = static_cast(testGraph.findNode("DetectMolecules")); + ASSERT_EQ(detectMoleculesNode->run(), NodeConstants::ProcessResult::Success); + + auto detectedMoleculeStructures = detectMoleculesNode->detectedStructures(); + EXPECT_EQ(detectedMoleculeStructures.size(), 1); + + auto &molStructure = detectedMoleculeStructures.front(); + EmpiricalFormula::EmpiricalFormulaMap moleculeFormula = { + {Elements::Cl, 1}, {Elements::O, 1}, {Elements::C, 1}, {Elements::H, 3}}; + testDetectedMolecularStructure(molStructure, {EmpiricalFormula::formula(moleculeFormula), 6, 6}); + + // auto &unitCellSpecies = static_cast(detectMoleculesNode)->cleanedUnitCellSpecies(); + // testInstanceConsistency(cifMolecule, unitCellSpecies); +} + +TEST_F(CIFNodeTest, MoleculeOrderingSimpleUnorderedRotated) +{ + TestGraph testGraph; + + auto cif = std::string("molecule-test-simple-unordered-rotated.cif"); + + EXPECT_TRUE(testGraph.appendNode("ImportCIFStructure")); + testGraph.fetchHead()->setOption("FilePath", "cif/" + cif); + ASSERT_TRUE(testGraph.appendNode("CalculateBonding")); + ASSERT_TRUE(testGraph.appendNode("DetectMolecules", "DetectMolecules")); + testGraph.addEdge({"ImportCIFStructure", "Structure", "CalculateBonding", "Structure"}); + testGraph.addEdge({"CalculateBonding", "Structure", "DetectMolecules", "Structure"}); + + auto detectMoleculesNode = static_cast(testGraph.findNode("DetectMolecules")); + ASSERT_EQ(detectMoleculesNode->run(), NodeConstants::ProcessResult::Success); + + auto detectedMoleculeStructures = detectMoleculesNode->detectedStructures(); + EXPECT_EQ(detectedMoleculeStructures.size(), 1); + + auto &molStructure = detectedMoleculeStructures.front(); + EmpiricalFormula::EmpiricalFormulaMap moleculeFormula = { + {Elements::Cl, 1}, {Elements::O, 1}, {Elements::C, 1}, {Elements::H, 3}}; + testDetectedMolecularStructure(molStructure, {EmpiricalFormula::formula(moleculeFormula), 6, 6}); + + // auto &unitCellSpecies = static_cast(detectMoleculesNode)->cleanedUnitCellSpecies(); + // testInstanceConsistency(cifMolecule, unitCellSpecies); } TEST_F(CIFNodeTest, BigMoleculeOrdering) { - const auto cifFile = "Bisphen_n_arenes_1517789.cif"; - createGraph(cifFile); - ASSERT_EQ(testGraph_.findNode(cifNameFromFile(cifFile))->run(), NodeConstants::ProcessResult::Success); + TestGraph testGraph; - auto cifContext = getContextByFileName(cifFile); - ASSERT_TRUE(cifContext); - EXPECT_TRUE(cifContext->generate()); + const auto cif = std::string("Bisphen_n_arenes_1517789.cif"); - auto molecularSpeciesNode = testGraph_.findNode(cifNameFromFile(cifFile) + "//MolecularSpecies"); - ASSERT_EQ(molecularSpeciesNode->run(), NodeConstants::ProcessResult::Success); + EXPECT_TRUE(testGraph.appendNode("ImportCIFStructure")); + testGraph.fetchHead()->setOption("FilePath", "cif/" + cif); + ASSERT_TRUE(testGraph.appendNode("DetectMolecules", "DetectMolecules")); + testGraph.addEdge({"ImportCIFStructure", "Structure", "DetectMolecules", "Structure"}); - auto molecularSpecies = molecularSpeciesNode->getOutputValue>("DetectedMolecularSpecies"); - EXPECT_EQ(molecularSpecies.size(), 1); + auto detectMoleculesNode = static_cast(testGraph.findNode("DetectMolecules")); + ASSERT_EQ(detectMoleculesNode->run(), NodeConstants::ProcessResult::Success); - auto &cifMolecule = molecularSpecies.front(); + auto detectedStructures = detectMoleculesNode->detectedStructures(); + EXPECT_EQ(detectedStructures.size(), 1); + + auto &molStructure = detectedStructures.front(); EmpiricalFormula::EmpiricalFormulaMap moleculeFormula = {{Elements::O, 6}, {Elements::C, 51}, {Elements::H, 54}}; - testMolecularSpecies(cifMolecule, {EmpiricalFormula::formula(moleculeFormula), 4, 111}); + testDetectedMolecularStructure(molStructure, {EmpiricalFormula::formula(moleculeFormula), 4, 111}); - auto &unitCellSpecies = static_cast(molecularSpeciesNode)->cleanedUnitCellSpecies(); - testInstanceConsistency(cifMolecule, unitCellSpecies); + // auto &unitCellSpecies = static_cast(detectMoleculesNode)->cleanedUnitCellSpecies(); + // testInstanceConsistency(cifMolecule, unitCellSpecies); } -*/ -} // namespace UnitTest + +} // namespace UnitTest \ No newline at end of file From 280518f74959f28657faae00578dad08de271ef9 Mon Sep 17 00:00:00 2001 From: RobBuchanan Date: Thu, 25 Jun 2026 11:59:18 +0100 Subject: [PATCH 2/8] clean up detect mols node --- src/nodes/detectMolecules.cpp | 68 ----------------------------------- src/nodes/detectMolecules.h | 13 ------- 2 files changed, 81 deletions(-) diff --git a/src/nodes/detectMolecules.cpp b/src/nodes/detectMolecules.cpp index 4e856fa6b7..cae00c5def 100644 --- a/src/nodes/detectMolecules.cpp +++ b/src/nodes/detectMolecules.cpp @@ -224,74 +224,6 @@ Structure &DetectMoleculesNode::copyStructureAtomsAndBonds(const Structure &sour return target; } -// Recursively check NETA description matches between the supplied atoms -std::map -DetectMoleculesNode::matchAtom(const StructureAtom *referenceAtom, const StructureAtom *instanceAtom, - const std::map &refNETA, - const std::map &map) -{ - // If the reference atom NETA doesn't match the instance atom we cannot proceed - if (!refNETA.at(referenceAtom).matches(instanceAtom)) - return {}; - - // Check the map to see if we have already associated the reference atom to an instance atom, or if the instance atom - // is already associated to a different reference atom. - for (auto &&[mappedRefAtom, mappedInstanceAtom] : map) - { - // Found it - double-check to ensure that the current association matches our instance atom. If it does we can return - // the map as it currently stands. If not we return an empty map to indicate failure. - if (mappedRefAtom == referenceAtom) - { - if (mappedInstanceAtom == instanceAtom) - { - return map; - } - else - { - return {}; - } - } - else if (mappedInstanceAtom == instanceAtom) - { - return {}; - } - } - - // Copy the current map, associate our initial pair of atoms and try to extend it - auto newMap = map; - newMap[referenceAtom] = instanceAtom; - - // Cycle over bonds on the reference atom and find - for (const auto &referenceBond : referenceAtom->bonds()) - { - // Get the reference bond partner - auto *referenceBondPartner = referenceBond->partner(referenceAtom); - - // Try to find a match over bonds on the instance atom - std::map bondResult; - for (const auto &instanceBond : instanceAtom->bonds()) - { - // Get the instance bond partner - auto *instanceBondPartner = instanceBond->partner(instanceAtom); - - // Recurse - bondResult = matchAtom(referenceBondPartner, instanceBondPartner, refNETA, newMap); - if (!bondResult.empty()) - break; - } - - // If we found a suitable match recursing into the bond, store the result into newMap and continue to the next bond. - // If we didn't find a good match, we return now. - if (bondResult.empty()) - return {}; - - newMap = bondResult; - } - - // If we get to here then we succeeded, so return the new map - return newMap; -} - /* * Getters */ diff --git a/src/nodes/detectMolecules.h b/src/nodes/detectMolecules.h index 3bcd80c341..57a8560308 100644 --- a/src/nodes/detectMolecules.h +++ b/src/nodes/detectMolecules.h @@ -35,8 +35,6 @@ class DetectMoleculesNode : public Node Structure inputStructure_; // Output structures std::vector detectedStructures_; - // Best NETA definitions for structure atoms - std::map bestNetaDefinitions_; /* * Helpers @@ -47,17 +45,6 @@ class DetectMoleculesNode : public Node const std::vector fragmentIndices); // Find molecular fragments std::vector> findMolecularFragments(const Structure &structure) const; - // Determine the best NETA definition for the supplied species - std::tuple> bestNETADefinition(const Structure &structure); - // Get instances for the supplied species from the cleaned unit cell - std::vector> getInstances(const Structure &referenceStructure, std::vector &atomMask, - const NETADefinition &neta, - const std::vector &referenceRootAtoms); - // Recursively check NETA description matches between the supplied atoms - std::map - matchAtom(const StructureAtom *referenceAtom, const StructureAtom *instanceAtom, - const std::map &refNETA, - const std::map &map); /* * Processing From 29740585d9ccf3b98d01fec8438ff73d4fafd2e7 Mon Sep 17 00:00:00 2001 From: RobBuchanan Date: Fri, 12 Jun 2026 12:26:13 +0100 Subject: [PATCH 3/8] dynamic outputs 1 --- src/nodes/node.cpp | 7 ++++ src/nodes/node.h | 40 +++++++++++++++++++++++ src/nodes/parameter.h | 1 + src/nodes/test.cpp | 14 ++++++++ src/nodes/test.h | 8 +++++ tests/graphData.h | 16 +++++++++ tests/nodes/parameters.cpp | 66 ++++++++++++++++++++++++++++++++++++++ 7 files changed, 152 insertions(+) diff --git a/src/nodes/node.cpp b/src/nodes/node.cpp index a566c7abbd..a926a9d5a8 100644 --- a/src/nodes/node.cpp +++ b/src/nodes/node.cpp @@ -140,6 +140,7 @@ NodeConstants::ProcessResult Node::run() case (NodeConstants::ProcessResult::Failed): break; case (NodeConstants::ProcessResult::Success): + updateDynamicOutputs(); ++versionIndex_; upToDate_ = true; break; @@ -282,6 +283,12 @@ std::shared_ptr Node::findOption(std::string_view optionName) con return options_.at(std::string{optionName}); } +// Update dynamic outputs +void Node::updateDynamicOutputs() { registerDynamicOutputs(); } + +// Register dynamic outputs +void Node::registerDynamicOutputs() {} + // Return Options Node::NodeParameterMap &Node::options() { return options_; }; diff --git a/src/nodes/node.h b/src/nodes/node.h index 662256b2b9..146008b79a 100644 --- a/src/nodes/node.h +++ b/src/nodes/node.h @@ -300,6 +300,46 @@ class Node : public Serialisable<> } // Return named option if it exists std::shared_ptr findOption(std::string_view name) const; + // Update dynamic outputs + void updateDynamicOutputs(); + // Register dynamic output from container data source + template + void registerDynamicOutput(std::vector &data, std::string description, + const std::variant> &name = std::string("dynOut")) + { + for (int i = 0; i < data.size(); i++) + { + auto val = data[i]; + auto paramName = std::holds_alternative(name) ? (std::get(name) + std::format("-{}", i)) + : std::get>(name)(val); + + // Check if output already exists - do not add if it does + if (outputs_.find(paramName) != outputs_.end()) + continue; + + addOutput(paramName, description, data[i]); + } + } + template + void + registerDynamicPointerOutput(std::vector &data, std::string description, + const std::variant> &name = std::string("dynOut")) + { + for (int i = 0; i < data.size(); i++) + { + auto val = data[i]; + auto paramName = std::holds_alternative(name) ? (std::get(name) + std::format("-{}", i)) + : std::get>(name)(val); + + // Check if output already exists - do not add if it does + if (outputs_.find(paramName) != outputs_.end()) + continue; + + addPointerOutput(paramName, description, data[i]); + } + } + // Register dynamic outputs + virtual void registerDynamicOutputs(); // Return options NodeParameterMap &options(); // Set option value diff --git a/src/nodes/parameter.h b/src/nodes/parameter.h index 805a118ce3..d0004ecfbd 100644 --- a/src/nodes/parameter.h +++ b/src/nodes/parameter.h @@ -53,6 +53,7 @@ class ParameterBase : public Serialisable<> ClearData, /* Indicates that any local data should be cleared if the parameter is changed */ Input, /* Indicates that the parameter is meant to be a sink for data and not a source */ Output, /* Indicates that the parameter is meant to be a source of data and not a sink */ + Dynamic, /* Indicates that the parameter is meant to be a dynamic source of data and not a sink */ }; // Allowed Edge Count enum AllowedEdgeCount diff --git a/src/nodes/test.cpp b/src/nodes/test.cpp index 3341632115..96ca88cb85 100644 --- a/src/nodes/test.cpp +++ b/src/nodes/test.cpp @@ -12,6 +12,9 @@ TestNode::TestNode(Graph *parentGraph) : Node(parentGraph) addInput("NumberVector", "A vector of numbers", numberVector_); addInput("OptionalNumber", "A single number", optionalNumber_); addInput("Variant", "A variant", variant_); + addInput("Message", "A message", message_); + addInput("Char", "A character", char_); + addInput("CharPtr", "A character", charPtr_); // Outputs addOutput("Configuration", "A configuration output", configuration_); @@ -42,6 +45,13 @@ TestNode::TestVariant TestNode::variant() { return variant_; } * Processing */ +// Register dynamic outputs +void TestNode::registerDynamicOutputs() +{ + registerDynamicOutput(messageParts_, "Individual character from a message", std::string("Message-Part")); + registerDynamicPointerOutput(messageParts_, "Individual character from a message", std::string("Message-Ptr-Part")); +} + // Perform processing NodeConstants::ProcessResult TestNode::process() { @@ -54,5 +64,9 @@ NodeConstants::ProcessResult TestNode::process() else optionalConfiguration_ = std::nullopt; + // Standard dynamic outputs + messageParts_.clear(); + messageParts_.insert(messageParts_.end(), message_.begin(), message_.end()); + return NodeConstants::ProcessResult::Success; } diff --git a/src/nodes/test.h b/src/nodes/test.h index 1cb3be09da..bc39cbe0eb 100644 --- a/src/nodes/test.h +++ b/src/nodes/test.h @@ -33,6 +33,11 @@ class TestNode : public Node // Variant using TestVariant = VariantParameterData; TestVariant variant_; + // Test string + char char_; + char *charPtr_; + std::string message_; + std::vector messageParts_; public: // Return type of the node @@ -53,6 +58,9 @@ class TestNode : public Node /* * Processing */ + private: + void registerDynamicOutputs() override; + protected: // Perform processing NodeConstants::ProcessResult process() override; diff --git a/tests/graphData.h b/tests/graphData.h index 8a551f23e5..8cbb670170 100644 --- a/tests/graphData.h +++ b/tests/graphData.h @@ -59,6 +59,22 @@ class TestGraph : public DissolveGraph std::string fetchHeadName() const { return head_ ? std::string(head_->name()) : "NO_NODE"; } // Returns reference to current top node in graph, cast to the known node type template NodeType *head() const { return static_cast(head_); } + // Run the graph in a piecewise manner - initially from a specific node, then from the last node - in order to emplace a set + // of dynamic edges that we expect to exist at run time + NodeConstants::ProcessResult runDynamic(Node *startNode, std::vector edges) + { + setUpdateRequired(); + + auto result = NodeConstants::ProcessResult::Unchanged; + result = startNode->run(); + if (result == NodeConstants::ProcessResult::Failed) + return result; + + for (const auto &edge : edges) + if (!addEdge(edge) || findNode(edge.targetNode)->run() == NodeConstants::ProcessResult::Failed) + return NodeConstants::ProcessResult::Failed; + return result; + } // Append new node to the graph Node *appendNode(const std::string &nodeType, const std::optional &name = {}) { diff --git a/tests/nodes/parameters.cpp b/tests/nodes/parameters.cpp index 96b9e6e0a1..4b479053f8 100644 --- a/tests/nodes/parameters.cpp +++ b/tests/nodes/parameters.cpp @@ -360,4 +360,70 @@ TEST_F(ParametersTest, OptionalPointerToVariant) EXPECT_EQ(std::get(b_->variant().data), &a_->optionalConfiguration().value()); } +TEST_F(ParametersTest, DynamicOutput) +{ + ASSERT_TRUE(testGraph_.addNode(std::make_unique(&testGraph_), "Sender")); + ASSERT_TRUE(testGraph_.addNode(std::make_unique(&testGraph_), "RecieverA")); + ASSERT_TRUE(testGraph_.addNode(std::make_unique(&testGraph_), "RecieverB")); + ASSERT_TRUE(testGraph_.addNode(std::make_unique(&testGraph_), "RecieverC")); + ASSERT_TRUE(testGraph_.addNode(std::make_unique(&testGraph_), "RecieverD")); + ASSERT_TRUE(testGraph_.addNode(std::make_unique(&testGraph_), "RecieverE")); + + auto sender = testGraph_.findNode("Sender"); + ASSERT_TRUE(sender->setInput("Message", std::string("hello"))); + ASSERT_EQ(testGraph_.runDynamic(sender, + { + {"Sender", "Message-Part-0", "RecieverA", "Char"}, + {"Sender", "Message-Part-1", "RecieverB", "Char"}, + {"Sender", "Message-Part-2", "RecieverC", "Char"}, + {"Sender", "Message-Part-3", "RecieverD", "Char"}, + {"Sender", "Message-Part-4", "RecieverE", "Char"}, + + }), + NodeConstants::ProcessResult::Success); + + std::vector chars; + for (const auto &which : {"A", "B", "C", "D", "E"}) + { + auto node = testGraph_.findNode("Reciever" + std::string(which)); + chars.push_back(node->findInput("Char")->get()); + } + + std::string message(chars.begin(), chars.end()); + ASSERT_EQ(message, "hello"); +} + +TEST_F(ParametersTest, DynamicPointerOutput) +{ + ASSERT_TRUE(testGraph_.addNode(std::make_unique(&testGraph_), "Sender")); + ASSERT_TRUE(testGraph_.addNode(std::make_unique(&testGraph_), "RecieverA")); + ASSERT_TRUE(testGraph_.addNode(std::make_unique(&testGraph_), "RecieverB")); + ASSERT_TRUE(testGraph_.addNode(std::make_unique(&testGraph_), "RecieverC")); + ASSERT_TRUE(testGraph_.addNode(std::make_unique(&testGraph_), "RecieverD")); + ASSERT_TRUE(testGraph_.addNode(std::make_unique(&testGraph_), "RecieverE")); + + auto sender = testGraph_.findNode("Sender"); + ASSERT_TRUE(sender->setInput("Message", std::string("hello"))); + ASSERT_EQ(testGraph_.runDynamic(sender, + { + {"Sender", "Message-Ptr-Part-0", "RecieverA", "CharPtr"}, + {"Sender", "Message-Ptr-Part-1", "RecieverB", "CharPtr"}, + {"Sender", "Message-Ptr-Part-2", "RecieverC", "CharPtr"}, + {"Sender", "Message-Ptr-Part-3", "RecieverD", "CharPtr"}, + {"Sender", "Message-Ptr-Part-4", "RecieverE", "CharPtr"}, + + }), + NodeConstants::ProcessResult::Success); + + std::vector chars; + for (const auto &which : {"A", "B", "C", "D", "E"}) + { + auto node = testGraph_.findNode("Reciever" + std::string(which)); + chars.push_back(*node->findInput("CharPtr")->get()); + } + + std::string message(chars.begin(), chars.end()); + ASSERT_EQ(message, "hello"); +} + } // namespace UnitTest From 5fec15707005d591be4786830431d0efbae288a4 Mon Sep 17 00:00:00 2001 From: RobBuchanan Date: Thu, 25 Jun 2026 14:08:42 +0100 Subject: [PATCH 4/8] start incorporating dynamic outputs to detect mols and get supercell config --- src/nodes/detectMolecules.cpp | 6 ++++++ src/nodes/detectMolecules.h | 3 +++ tests/nodes/cif.cpp | 28 ++++++++++++++++++++++++++-- 3 files changed, 35 insertions(+), 2 deletions(-) diff --git a/src/nodes/detectMolecules.cpp b/src/nodes/detectMolecules.cpp index cae00c5def..6b32130b89 100644 --- a/src/nodes/detectMolecules.cpp +++ b/src/nodes/detectMolecules.cpp @@ -31,6 +31,12 @@ std::vector> DetectMoleculesNode::findMolecularFragments(const return fragments; } +// Register dynamic outputs +void DetectMoleculesNode::registerDynamicOutputs() +{ + registerDynamicOutput(detectedStructures_, "Detect molecular structures", std::string("DetectedMolecule")); +} + // Run main processing NodeConstants::ProcessResult DetectMoleculesNode::process() { diff --git a/src/nodes/detectMolecules.h b/src/nodes/detectMolecules.h index 57a8560308..4c05d486be 100644 --- a/src/nodes/detectMolecules.h +++ b/src/nodes/detectMolecules.h @@ -50,6 +50,9 @@ class DetectMoleculesNode : public Node * Processing */ private: + void registerDynamicOutputs() override; + + protected: // Run main processing NodeConstants::ProcessResult process() override; diff --git a/tests/nodes/cif.cpp b/tests/nodes/cif.cpp index e9a32f688b..739ac57c00 100644 --- a/tests/nodes/cif.cpp +++ b/tests/nodes/cif.cpp @@ -216,8 +216,34 @@ TEST_F(CIFNodeTest, CuBTC) auto detectMoleculesNode = static_cast(testGraph.findNode("DetectMolecules")); ASSERT_EQ(detectMoleculesNode->run(), NodeConstants::ProcessResult::Success); + auto detectedMoleculeStructures = detectMoleculesNode->detectedStructures(); + EXPECT_EQ(detectedMoleculeStructures.size(), 2); + const auto box = detectedMoleculeStructures[0].box(); + EXPECT_EQ(testGraph.findNode("ImportCIFStructure")->findOption("SpaceGroupID")->get(), SpaceGroups::SpaceGroup_225); + + // Check box + EXPECT_TRUE(testGraph.appendNode("Configuration", "Box")); + EXPECT_TRUE(testGraph.appendNode("SetBox")); + EXPECT_TRUE(testGraph.appendNode("Species", "MolecularSpeciesA")); + EXPECT_TRUE(testGraph.appendNode("Species", "MolecularSpeciesB")); + EXPECT_TRUE(testGraph.appendNode("Insert", "InsertMolecularSpeciesA")); + EXPECT_TRUE(testGraph.appendNode("Insert", "InsertMolecularSpeciesB")); + EXPECT_TRUE(testGraph.appendNode("SupercellConfiguration")); + ASSERT_TRUE(testGraph.addEdge({"DetectMolecules", "DetectedMolecule-0", "MolecularSpeciesA", "Structure"})); + ASSERT_TRUE(testGraph.addEdge({"DetectMolecules", "DetectedMolecule-1", "MolecularSpeciesB", "Structure"})); + ASSERT_TRUE(testGraph.addEdge({"Box", "Configuration", "SetBox", "Input"})); + ASSERT_TRUE(testGraph.addEdge({"SetBox", "Output", "InsertMolecularSpeciesA", "Configuration"})); + ASSERT_TRUE(testGraph.addEdge({"MolecularSpeciesA", "Species", "InsertMolecularSpeciesA", "Species"})); + ASSERT_TRUE(testGraph.addEdge({"InsertMolecularSpeciesA", "Configuration", "InsertMolecularSpeciesB", "Configuration"})); + ASSERT_TRUE(testGraph.addEdge({"MolecularSpeciesB", "Species", "InsertMolecularSpeciesB", "Species"})); + ASSERT_TRUE(testGraph.addEdge({"InsertMolecularSpeciesB", "Configuration", "SupercellConfiguration", "Configuration"})); + + ASSERT_EQ(testGraph.fetchHead()->run(), NodeConstants::ProcessResult::Success); + + auto box = testGraph.fetchHead()->getOutputValue("SupercellConfiguration"); + constexpr auto A = 26.3336; // testBox(detectMoleculesNode->getOutputValue("SupercellConfiguration"), {A, A, A}, {90, 90, 90}, 672); @@ -231,8 +257,6 @@ TEST_F(CIFNodeTest, CuBTC) [](const auto &i) { return i.speciesAtom()->Z(); }), EmpiricalFormula::formula(cellFormulaH)); */ - auto detectedMoleculeStructureA = detectMoleculesNode->detectedStructures(); - EXPECT_EQ(detectedMoleculeStructureA.size(), 2); } /* TEST_F(CIFNodeTest, CuBTCActiveAssemblies) From 114de822da32ab6f91ee3cdf7ad5d4282524abe4 Mon Sep 17 00:00:00 2001 From: RobBuchanan Date: Fri, 26 Jun 2026 15:16:49 +0100 Subject: [PATCH 5/8] insert node can work on instances, supercell cif test cases - part 1 --- src/nodes/insert.cpp | 91 +++++++++++++++++++++++-- src/nodes/insert.h | 24 +++++-- tests/nodes/cif.cpp | 156 ++++++++++++++++++++++++++++++------------- 3 files changed, 215 insertions(+), 56 deletions(-) diff --git a/src/nodes/insert.cpp b/src/nodes/insert.cpp index 6460eacfb0..b3282e7580 100644 --- a/src/nodes/insert.cpp +++ b/src/nodes/insert.cpp @@ -8,20 +8,25 @@ #include "kernels/energy.h" #include "math/mathFunc.h" #include "nodes/dissolve.h" +#include +#include +#include InsertNode::InsertNode(Graph *parentGraph) : Node(parentGraph) { // Inputs addInput("Configuration", "Target configuration to insert into", configuration_); addOutput("Configuration", "Modified configuration", configuration_); - addInput("Species", "Species to add - all resulting molecules will have identical geometry", species_); - addInput("MoleculeSet", "MoleculeSet to use as the source", moleculeSet_); addInput("Population", "Population of the target to add", population_); addInput("Density", "Density at which to add the target", density_); + addInput("Species", "Source species or molecule set to add - all resulting molecules will have identical geometry", + speciesVariant_); + addInput("Instances", "", instances_); // Options addOption("DensityUnits", "Units of target density", densityUnits_); addOption("BoxAction", "Action to take on the Box geometry / volume on addition of the species", boxAction_); + addOption("InstantiationMethod", "Strategy for instantiation of species during insertion", instantiationMethod_); addOption("ScaleA", "Scale box length A when modifying volume", scaleA_); addOption("ScaleB", "Scale box length B when modifying volume", scaleB_); addOption("ScaleC", "Scale box length C when modifying volume", scaleC_); @@ -51,6 +56,18 @@ EnumOptions InsertNode::boxActionStyles() } EnumOptions getEnumOptions(InsertNode::BoxActionStyle) { return InsertNode::boxActionStyles(); } +// Return enum option info for InstantiationMethod +EnumOptions InsertNode::instantiationMethod() +{ + return EnumOptions("InstantiationMethod", + {{InsertNode::InstantiationMethod::Sample, "Sample"}, + {InsertNode::InstantiationMethod::InstantiateAll, "InstantiateAll"}}); +} +EnumOptions getEnumOptions(InsertNode::InstantiationMethod) +{ + return InsertNode::instantiationMethod(); +} + /* * Processing */ @@ -147,11 +164,17 @@ NodeConstants::ProcessResult InsertNode::process() { // Get target MoleculeSet MoleculeSet speciesMoleculeSet; - if (species_) - speciesMoleculeSet.addMolecule(species_); - const MoleculeSet &targetMoleculeSet = species_ ? speciesMoleculeSet : *moleculeSet_; + auto insertFromSpecies = speciesVariant_.isAlternative(std::type_index(typeid(const Species *))); + if (insertFromSpecies) + speciesMoleculeSet.addMolecule(std::get(speciesVariant_.data)); + const MoleculeSet &targetMoleculeSet = + insertFromSpecies ? speciesMoleculeSet : *std::get(speciesVariant_.data); + + // Bool flag - do we have instances for this species + auto hasInstances = !instances_.instances().empty(); - auto ipop = population_.asInteger(); + auto ipop = hasInstances && instantiationMethod_ == InstantiationMethod::InstantiateAll ? instances_.instances().size() + : population_.asInteger(); if (ipop <= 0) { warn("Population is zero so nothing will be added.\n"); @@ -175,6 +198,35 @@ NodeConstants::ProcessResult InsertNode::process() break; } + std::random_device rd; + std::mt19937 gen(rd()); + std::uniform_int_distribution<> distr(0, ipop - 1); + std::set alreadySampled; + + const auto sampleInstances = [&alreadySampled, &distr, &gen, ipop, this]() + { + const auto instances = this->instances_.instances(); + + int index = 0; + + // We've run out of new unsampled instances + if (alreadySampled.size() == instances.size()) + return -1; + + while (true) + { + auto sampledIndex = distr(gen); + if (alreadySampled.contains(sampledIndex)) + continue; + + index += sampledIndex; + alreadySampled.insert(index); + break; + } + + return index; + }; + Matrix3 transform; const auto &box = configuration_->box(); configuration_->atoms().reserve(configuration_->atoms().size() + nAnyAtoms); @@ -183,6 +235,33 @@ NodeConstants::ProcessResult InsertNode::process() // Add the Molecule auto mol = configuration_->copyMolecule(targetMoleculeSet.localMolecule(n)); + auto insertionComplete = false; + + // If we have instances, either instantiate all from current positions, or sample from them randomly and/or randomise + // position of Molecule over the whole box + if (hasInstances) + { + std::vector atomicCoords; + if (instantiationMethod_ == InstantiationMethod::InstantiateAll) + { + atomicCoords = instances_.instances()[n]; + insertionComplete = true; + } + else + { + auto instancesIndex = sampleInstances(); + if (instancesIndex < 0) + break; + } + + // Update molecular atomic coordinates + for (int i = 0; i < mol->nAtoms(); i++) + mol->atom(i)->setR(atomicCoords[i]); + + if (insertionComplete) + continue; + } + // Randomise position of Molecule over the whole box auto newCentre = box.getReal({DissolveMath::random(), DissolveMath::random(), DissolveMath::random()}); mol->setCentreOfGeometry(box, newCentre); diff --git a/src/nodes/insert.h b/src/nodes/insert.h index 587d4b88b1..bd4d2e8dd9 100644 --- a/src/nodes/insert.h +++ b/src/nodes/insert.h @@ -5,6 +5,7 @@ #include "base/units.h" #include "classes/moleculeSet.h" +#include "classes/structure.h" #include "nodes/node.h" class InsertNode : public Node @@ -36,19 +37,34 @@ class InsertNode : public Node // Return enum option info for BoxActionStyle static EnumOptions boxActionStyles(); + // Box Action Style + enum class InstantiationMethod + { + Sample, /* N instances sampled randomly from instances, honouring the specified rotation/translation options */ + InstantiateAll, /* Instantiate all M instances in their current positions */ + }; + // Return enum option info for BoxActionStyle + static EnumOptions instantiationMethod(); + private: + // Typedef for allowed insert types (species/moleculeset) + using InsertTypeVariant = VariantParameterData; + // Insert type input and output + InsertTypeVariant speciesVariant_; // Target configuration to insert into Configuration *configuration_{nullptr}; + // Instances + Structure instances_; // AtomTypes owned by the node const std::vector> *atomTypes_{nullptr}; - // Species to be added (if no MoleculeSet is given) - const Species *species_{nullptr}; - // MoleculeSet to be added (if no Species is given) - const MoleculeSet *moleculeSet_{nullptr}; // The default box action if none is specified static constexpr BoxActionStyle defaultBoxAction_ = BoxActionStyle::AddVolume; + // The default instantiation method if none is specified + static constexpr InstantiationMethod defaultInstantiationMethod_ = InstantiationMethod::InstantiateAll; // Action to take on the Box geometry / volume on addition of the species BoxActionStyle boxAction_{defaultBoxAction_}; + // Strategy for instantiation of species during insertion + InstantiationMethod instantiationMethod_{defaultInstantiationMethod_}; // Target density when adding molecules (if adjusting box size) Number density_{1.0}; // Units for the specified density value diff --git a/tests/nodes/cif.cpp b/tests/nodes/cif.cpp index 739ac57c00..025a879371 100644 --- a/tests/nodes/cif.cpp +++ b/tests/nodes/cif.cpp @@ -2,12 +2,15 @@ // Copyright (c) 2026 Team Dissolve and contributors #include "classes/empiricalFormula.h" +#include "data/elements.h" #include "nodes/calculateBonding.h" #include "nodes/cif/importCIFStructure.h" #include "nodes/detectMolecules.h" +#include "nodes/supercellConfiguration.h" #include "tests/graphData.h" #include "tests/testData.h" #include +#include #include namespace UnitTest @@ -21,6 +24,88 @@ class CIFNodeTest : public ::testing::Test public: // Molecular species information using MolecularSpeciesInfo = std::tuple; + // Extend graph to convert detected species to a supercell configuration + void extendToSupercell(TestGraph *graph, std::vector> expectedSpecies, + const Vector3 &boxLengths, const Vector3 &boxAngles, Vector3i supercellRepeat = {1, 1, 1}) + { + EXPECT_TRUE(graph->appendNode("Configuration")); + EXPECT_TRUE(graph->appendNode("SetBox")); + ASSERT_TRUE(graph->fetchHead()->setOption("Lengths", boxLengths)); + ASSERT_TRUE(graph->fetchHead()->setOption("Angles", boxAngles)); + EXPECT_TRUE(graph->appendNode("SupercellConfiguration")); + ASSERT_TRUE(graph->fetchHead()->setOption("SupercellRepeat", supercellRepeat)); + ASSERT_TRUE(graph->addEdge({"Configuration", "Configuration", "SetBox", "Input"})); + + const auto nExpectedSpecies = expectedSpecies.size(); + + for (const auto &sp : expectedSpecies) + { + auto [z, name] = sp; + EXPECT_TRUE(graph->addNode(TestGraph::createAtomicSpecies(z), name)); + EXPECT_TRUE(graph->appendNode("Insert", std::string("Insert" + name))); + } + + // Create species from structure + ASSERT_TRUE(graph->addEdge({"DetectMolecules", "DetectedMolecule-0", expectedSpecies.front().second, "Structure"})); + + // Pass configuration output from set box node to the input configuration of this insert node + ASSERT_TRUE( + graph->addEdge({"SetBox", "Output", std::string("Insert" + expectedSpecies.front().second), "Configuration"})); + + // Pass this species to its insert node + ASSERT_TRUE(graph->addEdge( + {expectedSpecies.front().second, "Species", std::string("Insert" + expectedSpecies.front().second), "Species"})); + + // Pass the corresponding detected molecular structure to this species' insert node + // TODO: check if we have a reliable molecule name to use here at the structure level + ASSERT_TRUE(graph->addEdge( + {"DetectMolecules", "DetectedMolecule-0", std::string("Insert" + expectedSpecies.front().second), "Instances"})); + + for (int i = 1; i < expectedSpecies.size() - 1; i++) + { + auto lastSpeciesName = expectedSpecies[i - 1].second; + auto speciesName = expectedSpecies[i].second; + + // Create species from structure + ASSERT_TRUE(graph->addEdge( + {"DetectMolecules", std::string("DetectedMolecule-" + std::to_string(i)), speciesName, "Structure"})); + + // Pass configuration output from preceding insert node to the input configuration of this one + ASSERT_TRUE(graph->addEdge({std::string("Insert" + lastSpeciesName), "Configuration", + std::string("Insert" + speciesName), "Configuration"})); + + // Pass this species to its insert node + ASSERT_TRUE(graph->addEdge({speciesName, "Species", std::string("Insert" + speciesName), "Species"})); + + // Pass the corresponding detected molecular structure to this species' insert node + // TODO: check if we have a reliable molecule name to use here at the structure level + ASSERT_TRUE(graph->addEdge({"DetectMolecules", std::string("DetectedMolecule-" + std::to_string(i)), + std::string("Insert" + speciesName), "Instances"})); + } + + // + ASSERT_TRUE(graph->addEdge({std::string("Insert" + expectedSpecies[nExpectedSpecies - 2].second), "Configuration", + std::string("Insert" + expectedSpecies.back().second), "Configuration"})); + + // Create species from structure + ASSERT_TRUE( + graph->addEdge({"DetectMolecules", std::string("DetectedMolecule-" + std::to_string(expectedSpecies.size() - 1)), + expectedSpecies.back().second, "Structure"})); + + // Pass configuration output from set box node to the input configuration of the supercell configuration + ASSERT_TRUE(graph->addEdge({std::string("Insert" + expectedSpecies.back().second), "Configuration", + "SupercellConfiguration", "Configuration"})); + + // Pass this species to its insert node + ASSERT_TRUE(graph->addEdge( + {expectedSpecies.back().second, "Species", std::string("Insert" + expectedSpecies.back().second), "Species"})); + + // Pass the corresponding detected molecular structure to this species' insert node + // TODO: check if we have a reliable molecule name to use here at the structure level + ASSERT_TRUE( + graph->addEdge({"DetectMolecules", std::string("DetectedMolecule-" + std::to_string(expectedSpecies.size() - 1)), + std::string("Insert" + expectedSpecies.back().second), "Instances"})); + } // Test Box definition void testBox(const Configuration *cfg, const Vector3 &lengths, const Vector3 &angles, int nAtoms) { @@ -55,11 +140,11 @@ class CIFNodeTest : public ::testing::Test // Locate the atom in the reference system at the instance atom coordinates auto instanceR = instanceAtom.r(); auto spAtomIt = std::find_if(referenceCoordinates.atoms().begin(), referenceCoordinates.atoms().end(), - [box, instanceR](const auto &refAtom) - { return box.minimumDistance(refAtom.r(), instanceR) < 0.01; }); + [box, instanceR](const auto &refAtom) + { return box.minimumDistance(refAtom.r(), instanceR) < 0.01; }); std::cout << std::format("{} {} {} {}", Elements::symbol(speciesAtom.Z()), instanceAtom.r().x, - instanceAtom.r().y, instanceAtom.r().z) - << std::endl; + instanceAtom.r().y, instanceAtom.r().z) + << std::endl; ASSERT_NE(spAtomIt, referenceCoordinates.atoms().end()); EXPECT_EQ(spAtomIt->Z(), speciesAtom.Z()); } @@ -140,13 +225,12 @@ TEST_F(CIFNodeTest, NaClMolecules) DissolveSystemTest::checkVec3(instance[0], (r2 - A / 2).abs()); // 2x2x2 supercell - /* TODO: Handle supercell configurations - detectMoleculesNode->setOption("SupercellRepeat", {2, 2, 2}); - testGraph.dissolveGraph()->setUpdateRequired(); - ASSERT_EQ(detectMoleculesNode->run(), NodeConstants::ProcessResult::Success); - testBox(detectMoleculesNode->getOutputValue("SupercellConfiguration"), {A * 2, A * 2, A * 2}, + extendToSupercell(&testGraph, {{Elements::Na, "Na"}, {Elements::Cl, "Cl"}}, {A * 2, A * 2, A * 2}, {90, 90, 90}, {2, 2, 2}); + auto supercellConfigurationNode = static_cast(testGraph.findNode("SupercellConfiguration")); + ASSERT_EQ(testGraph.findNode("SetBox")->run(), NodeConstants::ProcessResult::Success); + ASSERT_EQ(supercellConfigurationNode->run(), NodeConstants::ProcessResult::Success); + testBox(supercellConfigurationNode->getOutputValue("SupercellConfiguration"), {A * 2, A * 2, A * 2}, {90, 90, 90}, 8 * 8); - */ } TEST_F(CIFNodeTest, NaClO3) @@ -170,7 +254,8 @@ TEST_F(CIFNodeTest, NaClO3) SpaceGroups::SpaceGroup_198); constexpr double A = 6.55; // TODO: Handle supercell configurations - // testBox(detectMoleculesNode->getOutputValue("SupercellConfiguration"), {A, A, A}, {90, 90, 90}, 20); + // testBox(detectMoleculesNode->getOutputValue("SupercellConfiguration"), {A, A, A}, {90, 90, 90}, + // 20); // No bonding defs in the CIF, so we expect species for each atomic // component (4 Na, 4 Cl, and 12 O) @@ -223,37 +308,19 @@ TEST_F(CIFNodeTest, CuBTC) EXPECT_EQ(testGraph.findNode("ImportCIFStructure")->findOption("SpaceGroupID")->get(), SpaceGroups::SpaceGroup_225); - // Check box - EXPECT_TRUE(testGraph.appendNode("Configuration", "Box")); - EXPECT_TRUE(testGraph.appendNode("SetBox")); - EXPECT_TRUE(testGraph.appendNode("Species", "MolecularSpeciesA")); - EXPECT_TRUE(testGraph.appendNode("Species", "MolecularSpeciesB")); - EXPECT_TRUE(testGraph.appendNode("Insert", "InsertMolecularSpeciesA")); - EXPECT_TRUE(testGraph.appendNode("Insert", "InsertMolecularSpeciesB")); - EXPECT_TRUE(testGraph.appendNode("SupercellConfiguration")); - ASSERT_TRUE(testGraph.addEdge({"DetectMolecules", "DetectedMolecule-0", "MolecularSpeciesA", "Structure"})); - ASSERT_TRUE(testGraph.addEdge({"DetectMolecules", "DetectedMolecule-1", "MolecularSpeciesB", "Structure"})); - ASSERT_TRUE(testGraph.addEdge({"Box", "Configuration", "SetBox", "Input"})); - ASSERT_TRUE(testGraph.addEdge({"SetBox", "Output", "InsertMolecularSpeciesA", "Configuration"})); - ASSERT_TRUE(testGraph.addEdge({"MolecularSpeciesA", "Species", "InsertMolecularSpeciesA", "Species"})); - ASSERT_TRUE(testGraph.addEdge({"InsertMolecularSpeciesA", "Configuration", "InsertMolecularSpeciesB", "Configuration"})); - ASSERT_TRUE(testGraph.addEdge({"MolecularSpeciesB", "Species", "InsertMolecularSpeciesB", "Species"})); - ASSERT_TRUE(testGraph.addEdge({"InsertMolecularSpeciesB", "Configuration", "SupercellConfiguration", "Configuration"})); - - ASSERT_EQ(testGraph.fetchHead()->run(), NodeConstants::ProcessResult::Success); - - auto box = testGraph.fetchHead()->getOutputValue("SupercellConfiguration"); - + /* TODO: Handle supercell configurations constexpr auto A = 26.3336; - // testBox(detectMoleculesNode->getOutputValue("SupercellConfiguration"), {A, A, A}, {90, 90, 90}, 672); + // testBox(detectMoleculesNode->getOutputValue("SupercellConfiguration"), {A, A, A}, {90, 90, 90}, + 672); // Check basic formula (which includes bound water oxygens - with no H - at this point) and using O group - /* TODO: Handle supercell configurations + // 16 basic formula units per unit cell constexpr auto N = 16; EmpiricalFormula::EmpiricalFormulaMap cellFormulaH = { {Elements::Cu, 3 * N}, {Elements::C, 18 * N}, {Elements::H, 6 * N}, {Elements::O, 15 * N}}; - EXPECT_EQ(EmpiricalFormula::formula(detectMoleculesNode->getOutputValue("SupercellConfiguration")->atoms(), + EXPECT_EQ(EmpiricalFormula::formula(detectMoleculesNode->getOutputValue("SupercellConfiguration")->atoms(), [](const auto &i) { return i.speciesAtom()->Z(); }), EmpiricalFormula::formula(cellFormulaH)); */ @@ -280,18 +347,15 @@ TEST_F(CIFNodeTest, CuBTCActiveAssemblies) testGraph.fetchHead()->setOption("AtomGroup", std::string("2")); testGraph.fetchHead()->setOption("SetActive", true); testGraph.removeEdge( - {cifNameFromFile(cif) + "//StructureCleanup", "CIFContext", std::string(detectMoleculesNode->name()), "CIFContext"}); - testGraph.addEdge( - {cifNameFromFile(cif) + "//StructureCleanup", "CIFContext", cifNameFromFile(cif) + "//AtomGroupA1", "CIFContext"}); - testGraph.addEdge( - {cifNameFromFile(cif) + "//AtomGroupA1", "CIFContext", cifNameFromFile(cif) + "//AtomGroupB2", "CIFContext"}); - testGraph.addEdge( - {cifNameFromFile(cif) + "//AtomGroupB2", "CIFContext", cifNameFromFile(cif) + "//AtomGroupC2", "CIFContext"}); - testGraph.addEdge( - {cifNameFromFile(cif) + "//AtomGroupC2", "CIFContext", std::string(detectMoleculesNode->name()), "CIFContext"}); - testGraph.setUpdateRequired(); + {cifNameFromFile(cif) + "//StructureCleanup", "CIFContext", std::string(detectMoleculesNode->name()), +"CIFContext"}); testGraph.addEdge( {cifNameFromFile(cif) + "//StructureCleanup", "CIFContext", cifNameFromFile(cif) + +"//AtomGroupA1", "CIFContext"}); testGraph.addEdge( {cifNameFromFile(cif) + "//AtomGroupA1", "CIFContext", +cifNameFromFile(cif) + "//AtomGroupB2", "CIFContext"}); testGraph.addEdge( {cifNameFromFile(cif) + "//AtomGroupB2", +"CIFContext", cifNameFromFile(cif) + "//AtomGroupC2", "CIFContext"}); testGraph.addEdge( {cifNameFromFile(cif) + +"//AtomGroupC2", "CIFContext", std::string(detectMoleculesNode->name()), "CIFContext"}); testGraph.setUpdateRequired(); ASSERT_EQ(detectMoleculesNode->run(), NodeConstants::ProcessResult::Success); - EXPECT_EQ(EmpiricalFormula::formula(detectMoleculesNode->getOutputValue("SupercellConfiguration")->atoms(), + EXPECT_EQ(EmpiricalFormula::formula(detectMoleculesNode->getOutputValue("SupercellConfiguration")->atoms(), [](const auto &i) { return i.speciesAtom()->Z(); }), EmpiricalFormula::formula(cellFormulaNH2)); From 6a17457fc8ffc841869ada1bf0277975009f754d Mon Sep 17 00:00:00 2001 From: RobBuchanan Date: Fri, 26 Jun 2026 15:28:42 +0100 Subject: [PATCH 6/8] nacl mols passes with supercell test --- tests/nodes/cif.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/nodes/cif.cpp b/tests/nodes/cif.cpp index 025a879371..4952ebcb0c 100644 --- a/tests/nodes/cif.cpp +++ b/tests/nodes/cif.cpp @@ -43,6 +43,7 @@ class CIFNodeTest : public ::testing::Test auto [z, name] = sp; EXPECT_TRUE(graph->addNode(TestGraph::createAtomicSpecies(z), name)); EXPECT_TRUE(graph->appendNode("Insert", std::string("Insert" + name))); + ASSERT_TRUE(graph->fetchHead()->setOption("BoxAction", InsertNode::BoxActionStyle::None)); } // Create species from structure @@ -225,7 +226,7 @@ TEST_F(CIFNodeTest, NaClMolecules) DissolveSystemTest::checkVec3(instance[0], (r2 - A / 2).abs()); // 2x2x2 supercell - extendToSupercell(&testGraph, {{Elements::Na, "Na"}, {Elements::Cl, "Cl"}}, {A * 2, A * 2, A * 2}, {90, 90, 90}, {2, 2, 2}); + extendToSupercell(&testGraph, {{Elements::Na, "Na"}, {Elements::Cl, "Cl"}}, {A, A, A}, {90, 90, 90}, {2, 2, 2}); auto supercellConfigurationNode = static_cast(testGraph.findNode("SupercellConfiguration")); ASSERT_EQ(testGraph.findNode("SetBox")->run(), NodeConstants::ProcessResult::Success); ASSERT_EQ(supercellConfigurationNode->run(), NodeConstants::ProcessResult::Success); From cf5b19dc697e7fa80d2b65d9f3a77a5d28e22ae3 Mon Sep 17 00:00:00 2001 From: RobBuchanan Date: Fri, 26 Jun 2026 17:19:30 +0100 Subject: [PATCH 7/8] good progress on cif tests and supercell box checking in most neccesary cases --- tests/nodes/cif.cpp | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/tests/nodes/cif.cpp b/tests/nodes/cif.cpp index 4952ebcb0c..30ea07be9f 100644 --- a/tests/nodes/cif.cpp +++ b/tests/nodes/cif.cpp @@ -228,7 +228,6 @@ TEST_F(CIFNodeTest, NaClMolecules) // 2x2x2 supercell extendToSupercell(&testGraph, {{Elements::Na, "Na"}, {Elements::Cl, "Cl"}}, {A, A, A}, {90, 90, 90}, {2, 2, 2}); auto supercellConfigurationNode = static_cast(testGraph.findNode("SupercellConfiguration")); - ASSERT_EQ(testGraph.findNode("SetBox")->run(), NodeConstants::ProcessResult::Success); ASSERT_EQ(supercellConfigurationNode->run(), NodeConstants::ProcessResult::Success); testBox(supercellConfigurationNode->getOutputValue("SupercellConfiguration"), {A * 2, A * 2, A * 2}, {90, 90, 90}, 8 * 8); @@ -253,10 +252,6 @@ TEST_F(CIFNodeTest, NaClO3) EXPECT_EQ(testGraph.findNode("ImportCIFStructure")->findOption("SpaceGroupID")->get(), SpaceGroups::SpaceGroup_198); - constexpr double A = 6.55; - // TODO: Handle supercell configurations - // testBox(detectMoleculesNode->getOutputValue("SupercellConfiguration"), {A, A, A}, {90, 90, 90}, - // 20); // No bonding defs in the CIF, so we expect species for each atomic // component (4 Na, 4 Cl, and 12 O) @@ -267,6 +262,13 @@ TEST_F(CIFNodeTest, NaClO3) testDetectedMolecularStructure(detectedMoleculeStructureA.at(1), {"Cl", 4, 1}); testDetectedMolecularStructure(detectedMoleculeStructureA.at(2), {"O", 12, 1}); + // Check box + constexpr double A = 6.55; + extendToSupercell(&testGraph, {{Elements::Na, "Na"}, {Elements::Cl, "Cl"}, {Elements::O, "O"}}, {A, A, A}, {90, 90, 90}); + auto supercellConfigurationNode = static_cast(testGraph.findNode("SupercellConfiguration")); + ASSERT_EQ(supercellConfigurationNode->run(), NodeConstants::ProcessResult::Success); + testBox(supercellConfigurationNode->getOutputValue("SupercellConfiguration"), {A, A, A}, {90, 90, 90}, 20); + // Calculate bonding ourselves to get the correct species EXPECT_TRUE(testGraph.appendNode("CalculateBonding")); testGraph.removeEdge({"ImportCIFStructure", "Structure", "DetectMolecules", "Structure"}); From d5d2f9001ae6b0c544f1377f6c05da40389e6d05 Mon Sep 17 00:00:00 2001 From: RobBuchanan Date: Fri, 26 Jun 2026 18:18:47 +0100 Subject: [PATCH 8/8] refactor dynamic outputs --- src/nodes/detectMolecules.cpp | 30 ++++++++++++--------- src/nodes/detectMolecules.h | 10 ------- src/nodes/node.cpp | 7 ----- src/nodes/node.h | 40 --------------------------- src/nodes/test.cpp | 37 ++++++++++++++++++++----- src/nodes/test.h | 3 --- tests/nodes/cif.cpp | 51 +++++++++++++++++++++-------------- 7 files changed, 78 insertions(+), 100 deletions(-) diff --git a/src/nodes/detectMolecules.cpp b/src/nodes/detectMolecules.cpp index 6b32130b89..66c58bab28 100644 --- a/src/nodes/detectMolecules.cpp +++ b/src/nodes/detectMolecules.cpp @@ -31,12 +31,6 @@ std::vector> DetectMoleculesNode::findMolecularFragments(const return fragments; } -// Register dynamic outputs -void DetectMoleculesNode::registerDynamicOutputs() -{ - registerDynamicOutput(detectedStructures_, "Detect molecular structures", std::string("DetectedMolecule")); -} - // Run main processing NodeConstants::ProcessResult DetectMoleculesNode::process() { @@ -195,6 +189,23 @@ NodeConstants::ProcessResult DetectMoleculesNode::process() EmpiricalFormula::formula(structure.atoms(), [](const auto &i) { return i->Z(); })); message(""); + /* + * Dynamic outputs + */ + + // Register dynamic outputs + for (int i = 0; i < detectedStructures_.size(); i++) + { + auto val = detectedStructures_[i]; + auto paramName = std::string("DetectedMolecule" + std::format("-{}", i)); + + // Check if output already exists - do not add if it does + if (outputs_.find(paramName) != outputs_.end()) + continue; + + addOutput(paramName, "Detected molecular structure", detectedStructures_[i]); + } + return NodeConstants::ProcessResult::Success; } @@ -229,10 +240,3 @@ Structure &DetectMoleculesNode::copyStructureAtomsAndBonds(const Structure &sour return target; } - -/* - * Getters - */ - -// Output structures -const std::vector &DetectMoleculesNode::detectedStructures() const { return detectedStructures_; } \ No newline at end of file diff --git a/src/nodes/detectMolecules.h b/src/nodes/detectMolecules.h index 4c05d486be..cc334435ac 100644 --- a/src/nodes/detectMolecules.h +++ b/src/nodes/detectMolecules.h @@ -49,17 +49,7 @@ class DetectMoleculesNode : public Node /* * Processing */ - private: - void registerDynamicOutputs() override; - protected: // Run main processing NodeConstants::ProcessResult process() override; - - /* - * Getters - */ - public: - // Output structures - const std::vector &detectedStructures() const; }; diff --git a/src/nodes/node.cpp b/src/nodes/node.cpp index a926a9d5a8..a566c7abbd 100644 --- a/src/nodes/node.cpp +++ b/src/nodes/node.cpp @@ -140,7 +140,6 @@ NodeConstants::ProcessResult Node::run() case (NodeConstants::ProcessResult::Failed): break; case (NodeConstants::ProcessResult::Success): - updateDynamicOutputs(); ++versionIndex_; upToDate_ = true; break; @@ -283,12 +282,6 @@ std::shared_ptr Node::findOption(std::string_view optionName) con return options_.at(std::string{optionName}); } -// Update dynamic outputs -void Node::updateDynamicOutputs() { registerDynamicOutputs(); } - -// Register dynamic outputs -void Node::registerDynamicOutputs() {} - // Return Options Node::NodeParameterMap &Node::options() { return options_; }; diff --git a/src/nodes/node.h b/src/nodes/node.h index 146008b79a..662256b2b9 100644 --- a/src/nodes/node.h +++ b/src/nodes/node.h @@ -300,46 +300,6 @@ class Node : public Serialisable<> } // Return named option if it exists std::shared_ptr findOption(std::string_view name) const; - // Update dynamic outputs - void updateDynamicOutputs(); - // Register dynamic output from container data source - template - void registerDynamicOutput(std::vector &data, std::string description, - const std::variant> &name = std::string("dynOut")) - { - for (int i = 0; i < data.size(); i++) - { - auto val = data[i]; - auto paramName = std::holds_alternative(name) ? (std::get(name) + std::format("-{}", i)) - : std::get>(name)(val); - - // Check if output already exists - do not add if it does - if (outputs_.find(paramName) != outputs_.end()) - continue; - - addOutput(paramName, description, data[i]); - } - } - template - void - registerDynamicPointerOutput(std::vector &data, std::string description, - const std::variant> &name = std::string("dynOut")) - { - for (int i = 0; i < data.size(); i++) - { - auto val = data[i]; - auto paramName = std::holds_alternative(name) ? (std::get(name) + std::format("-{}", i)) - : std::get>(name)(val); - - // Check if output already exists - do not add if it does - if (outputs_.find(paramName) != outputs_.end()) - continue; - - addPointerOutput(paramName, description, data[i]); - } - } - // Register dynamic outputs - virtual void registerDynamicOutputs(); // Return options NodeParameterMap &options(); // Set option value diff --git a/src/nodes/test.cpp b/src/nodes/test.cpp index 96ca88cb85..7dc5e54568 100644 --- a/src/nodes/test.cpp +++ b/src/nodes/test.cpp @@ -45,13 +45,6 @@ TestNode::TestVariant TestNode::variant() { return variant_; } * Processing */ -// Register dynamic outputs -void TestNode::registerDynamicOutputs() -{ - registerDynamicOutput(messageParts_, "Individual character from a message", std::string("Message-Part")); - registerDynamicPointerOutput(messageParts_, "Individual character from a message", std::string("Message-Ptr-Part")); -} - // Perform processing NodeConstants::ProcessResult TestNode::process() { @@ -68,5 +61,35 @@ NodeConstants::ProcessResult TestNode::process() messageParts_.clear(); messageParts_.insert(messageParts_.end(), message_.begin(), message_.end()); + /* + * Dynamic outputs + */ + + // Register dynamic (standard) outputs + for (int i = 0; i < messageParts_.size(); i++) + { + auto val = messageParts_[i]; + auto paramName = std::string("Message-Part" + std::format("-{}", i)); + + // Check if output already exists - do not add if it does + if (outputs_.find(paramName) != outputs_.end()) + continue; + + addOutput(paramName, "Part of a message", messageParts_[i]); + } + + // Register dynamic pointer outputs + for (int i = 0; i < messageParts_.size(); i++) + { + auto val = messageParts_[i]; + auto paramName = std::string("Message-Ptr-Part" + std::format("-{}", i)); + + // Check if output already exists - do not add if it does + if (outputs_.find(paramName) != outputs_.end()) + continue; + + addPointerOutput(paramName, "Part of a message", messageParts_[i]); + } + return NodeConstants::ProcessResult::Success; } diff --git a/src/nodes/test.h b/src/nodes/test.h index bc39cbe0eb..fe7d3be181 100644 --- a/src/nodes/test.h +++ b/src/nodes/test.h @@ -58,9 +58,6 @@ class TestNode : public Node /* * Processing */ - private: - void registerDynamicOutputs() override; - protected: // Perform processing NodeConstants::ProcessResult process() override; diff --git a/tests/nodes/cif.cpp b/tests/nodes/cif.cpp index 30ea07be9f..764a724923 100644 --- a/tests/nodes/cif.cpp +++ b/tests/nodes/cif.cpp @@ -24,6 +24,17 @@ class CIFNodeTest : public ::testing::Test public: // Molecular species information using MolecularSpeciesInfo = std::tuple; + // Retrieve detected molecule structures + std::vector getDetectedMolecularStructures(const DetectMoleculesNode *node, int N) + { + std::vector structures; + for (int i = 0; i < N; i++) + { + auto structureI = node->findOutput("DetectedMolecule-" + std::to_string(i)); + structures.push_back(structureI->get()); + } + return structures; + } // Extend graph to convert detected species to a supercell configuration void extendToSupercell(TestGraph *graph, std::vector> expectedSpecies, const Vector3 &boxLengths, const Vector3 &boxAngles, Vector3i supercellRepeat = {1, 1, 1}) @@ -216,13 +227,13 @@ TEST_F(CIFNodeTest, NaClMolecules) SpaceGroups::SpaceGroup_225); constexpr double A = 5.62; - EXPECT_EQ(detectMoleculesNode->detectedStructures().size(), 2); - testDetectedMolecularStructure(detectMoleculesNode->detectedStructures().at(0), {"Na", 4, 1}); + EXPECT_EQ(getDetectedMolecularStructures(detectMoleculesNode, 2).size(), 2); + testDetectedMolecularStructure(getDetectedMolecularStructures(detectMoleculesNode, 2).at(0), {"Na", 4, 1}); std::vector R = {{0.0, 0.0, 0.0}, {0.0, A / 2, A / 2}, {A / 2, 0.0, A / 2}, {A / 2, A / 2, 0.0}}; - for (auto &&[instance, r2] : zip(detectMoleculesNode->detectedStructures().at(0).instances(), R)) + for (auto &&[instance, r2] : zip(getDetectedMolecularStructures(detectMoleculesNode, 2).at(0).instances(), R)) DissolveSystemTest::checkVec3(instance[0], r2); - testDetectedMolecularStructure(detectMoleculesNode->detectedStructures().at(1), {"Cl", 4, 1}); - for (auto &&[instance, r2] : zip(detectMoleculesNode->detectedStructures().at(1).instances(), R)) + testDetectedMolecularStructure(getDetectedMolecularStructures(detectMoleculesNode, 2).at(1), {"Cl", 4, 1}); + for (auto &&[instance, r2] : zip(getDetectedMolecularStructures(detectMoleculesNode, 2).at(1).instances(), R)) DissolveSystemTest::checkVec3(instance[0], (r2 - A / 2).abs()); // 2x2x2 supercell @@ -257,10 +268,10 @@ TEST_F(CIFNodeTest, NaClO3) // component (4 Na, 4 Cl, and 12 O) ASSERT_EQ(detectMoleculesNode->run(), NodeConstants::ProcessResult::Success); - auto detectedMoleculeStructureA = detectMoleculesNode->detectedStructures(); - testDetectedMolecularStructure(detectedMoleculeStructureA.at(0), {"Na", 4, 1}); - testDetectedMolecularStructure(detectedMoleculeStructureA.at(1), {"Cl", 4, 1}); - testDetectedMolecularStructure(detectedMoleculeStructureA.at(2), {"O", 12, 1}); + auto detectedMoleculeStructuresA = getDetectedMolecularStructures(detectMoleculesNode, 3); + testDetectedMolecularStructure(detectedMoleculeStructuresA.at(0), {"Na", 4, 1}); + testDetectedMolecularStructure(detectedMoleculeStructuresA.at(1), {"Cl", 4, 1}); + testDetectedMolecularStructure(detectedMoleculeStructuresA.at(2), {"O", 12, 1}); // Check box constexpr double A = 6.55; @@ -277,10 +288,10 @@ TEST_F(CIFNodeTest, NaClO3) testGraph.setUpdateRequired(); ASSERT_EQ(detectMoleculesNode->run(), NodeConstants::ProcessResult::Success); - auto detectedMoleculeStructureB = detectMoleculesNode->detectedStructures(); - ASSERT_EQ(detectedMoleculeStructureB.size(), 2); - testDetectedMolecularStructure(detectedMoleculeStructureB.at(0), {"Na", 4, 1}); - testDetectedMolecularStructure(detectedMoleculeStructureB.at(1), {"ClO3", 4, 4}); + auto detectedMoleculeStructuresB = getDetectedMolecularStructures(detectMoleculesNode, 2); + ASSERT_EQ(detectedMoleculeStructuresB.size(), 2); + testDetectedMolecularStructure(detectedMoleculeStructuresB.at(0), {"Na", 4, 1}); + testDetectedMolecularStructure(detectedMoleculeStructuresB.at(1), {"ClO3", 4, 4}); } TEST_F(CIFNodeTest, CuBTC) @@ -304,7 +315,7 @@ TEST_F(CIFNodeTest, CuBTC) auto detectMoleculesNode = static_cast(testGraph.findNode("DetectMolecules")); ASSERT_EQ(detectMoleculesNode->run(), NodeConstants::ProcessResult::Success); - auto detectedMoleculeStructures = detectMoleculesNode->detectedStructures(); + auto detectedMoleculeStructures = getDetectedMolecularStructures(detectMoleculesNode, 2); EXPECT_EQ(detectedMoleculeStructures.size(), 2); const auto box = detectedMoleculeStructures[0].box(); @@ -367,8 +378,8 @@ cifNameFromFile(cif) + "//AtomGroupB2", "CIFContext"}); testGraph.addEdge( {cifN removeAtomicsNode->setOption("RemoveAtomics", true); testGraph.setUpdateRequired(); ASSERT_EQ(detectMoleculesNode->run(), NodeConstants::ProcessResult::Success); - auto detectedMoleculeStructureB = detectMoleculesNode->detectedStructures(); - EXPECT_EQ(detectedMoleculeStructureB.size(), 0); + auto detectedMoleculeStructuresB = getDetectedMolecularStructures(detectMoleculesNode, 2); + EXPECT_EQ(detectedMoleculeStructuresB.size(), 0); } */ @@ -389,7 +400,7 @@ TEST_F(CIFNodeTest, MoleculeOrderingSimple) auto detectMoleculesNode = static_cast(testGraph.findNode("DetectMolecules")); ASSERT_EQ(detectMoleculesNode->run(), NodeConstants::ProcessResult::Success); - auto detectedMoleculeStructures = detectMoleculesNode->detectedStructures(); + auto detectedMoleculeStructures = getDetectedMolecularStructures(detectMoleculesNode, 1); EXPECT_EQ(detectedMoleculeStructures.size(), 1); auto &molStructure = detectedMoleculeStructures.front(); @@ -417,7 +428,7 @@ TEST_F(CIFNodeTest, MoleculeOrderingSimpleUnordered) auto detectMoleculesNode = static_cast(testGraph.findNode("DetectMolecules")); ASSERT_EQ(detectMoleculesNode->run(), NodeConstants::ProcessResult::Success); - auto detectedMoleculeStructures = detectMoleculesNode->detectedStructures(); + auto detectedMoleculeStructures = getDetectedMolecularStructures(detectMoleculesNode, 1); EXPECT_EQ(detectedMoleculeStructures.size(), 1); auto &molStructure = detectedMoleculeStructures.front(); @@ -445,7 +456,7 @@ TEST_F(CIFNodeTest, MoleculeOrderingSimpleUnorderedRotated) auto detectMoleculesNode = static_cast(testGraph.findNode("DetectMolecules")); ASSERT_EQ(detectMoleculesNode->run(), NodeConstants::ProcessResult::Success); - auto detectedMoleculeStructures = detectMoleculesNode->detectedStructures(); + auto detectedMoleculeStructures = getDetectedMolecularStructures(detectMoleculesNode, 1); EXPECT_EQ(detectedMoleculeStructures.size(), 1); auto &molStructure = detectedMoleculeStructures.front(); @@ -471,7 +482,7 @@ TEST_F(CIFNodeTest, BigMoleculeOrdering) auto detectMoleculesNode = static_cast(testGraph.findNode("DetectMolecules")); ASSERT_EQ(detectMoleculesNode->run(), NodeConstants::ProcessResult::Success); - auto detectedStructures = detectMoleculesNode->detectedStructures(); + auto detectedStructures = getDetectedMolecularStructures(detectMoleculesNode, 1); EXPECT_EQ(detectedStructures.size(), 1); auto &molStructure = detectedStructures.front();