Skip to content

Commit

Permalink
cml format lint fix and graph ADL removal
Browse files Browse the repository at this point in the history 
Signed-off-by: Nathan Young <[email protected]>
  • Loading branch information
@TactfulDeity
TactfulDeity committed Nov 14, 2024
1 parent 2854c6d commit dd05561
Show file tree
Hide file tree
Showing 2 changed files with 90 additions and 86 deletions.
41 changes: 15 additions & 26 deletions avogadro/core/graph.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include <cassert>
#include <set>
#include <stack>
#include <utility>

namespace Avogadro::Core {

Expand Down Expand Up @@ -70,9 +71,6 @@ size_t Graph::addVertex()

void Graph::removeVertex(size_t index)
{
// Allow ADL for swap
using std::swap;

assert(index < size());
// Mark the subgraph as dirty, leave the work for later
if (m_vertexToSubgraph[index] >= 0)
Expand All @@ -82,7 +80,7 @@ void Graph::removeVertex(size_t index)

// Swap with last vertex.
if (index < size() - 1) {
swap(m_adjacencyList[index], m_adjacencyList.back());
std::swap(m_adjacencyList[index], m_adjacencyList.back());
size_t affectedIndex = m_adjacencyList.size() - 1;
// NOLINTBEGIN(*)
for (size_t i = 0; i < m_adjacencyList[index].size(); i++) {
Expand All @@ -93,7 +91,7 @@ void Graph::removeVertex(size_t index)
}
}
// NOLINTEND(*)
swap(m_edgeMap[index], m_edgeMap.back());
std::swap(m_edgeMap[index], m_edgeMap.back());
for (size_t i = 0; i < m_edgeMap[index].size(); i++) {
size_t edgeIndex = m_edgeMap[index][i];
if (m_edgePairs[edgeIndex].first == affectedIndex)
Expand All @@ -103,7 +101,7 @@ void Graph::removeVertex(size_t index)
}
if (m_vertexToSubgraph[index] >= 0)
m_subgraphToVertices[m_vertexToSubgraph[index]].erase(index);
swap(m_vertexToSubgraph[index], m_vertexToSubgraph.back());
std::swap(m_vertexToSubgraph[index], m_vertexToSubgraph.back());
if (m_vertexToSubgraph[index] >= 0) {
m_subgraphToVertices[m_vertexToSubgraph[index]].erase(affectedIndex);
m_subgraphToVertices[m_vertexToSubgraph[index]].insert(index);
Expand Down Expand Up @@ -144,7 +142,7 @@ void Graph::swapVertexIndices(size_t a, size_t b)
// NOLINTEND(*)
}

swap(m_adjacencyList[a], m_adjacencyList[b]);
std::swap(m_adjacencyList[a], m_adjacencyList[b]);

// Update m_edgePairs using info from m_edgeMap
for (size_t i = 0; i < m_edgeMap[a].size(); i++) {
Expand All @@ -168,7 +166,7 @@ void Graph::swapVertexIndices(size_t a, size_t b)
m_edgePairs[edgeIndex].second = a;
}

swap(m_edgeMap[a], m_edgeMap[b]);
std::swap(m_edgeMap[a], m_edgeMap[b]);
}

size_t Graph::vertexCount() const
Expand All @@ -178,13 +176,10 @@ size_t Graph::vertexCount() const

size_t Graph::addEdge(size_t a, size_t b)
{
// Allow ADL for swap
using std::swap;

assert(a < size());
assert(b < size());
if (b < a)
swap(a, b);
std::swap(a, b);
std::vector<size_t>& neighborsA = m_adjacencyList[a];
std::vector<size_t>& neighborsB = m_adjacencyList[b];

Expand Down Expand Up @@ -281,9 +276,6 @@ std::set<size_t> Graph::checkConectivity(size_t a, size_t b) const

void Graph::removeEdge(size_t a, size_t b)
{
// Allow ADL for swap
using std::swap;

assert(a < size());
assert(b < size());

Expand All @@ -295,31 +287,31 @@ void Graph::removeEdge(size_t a, size_t b)
if (iter == neighborsA.end())
return;

swap(*iter, neighborsA.back());
std::swap(*iter, neighborsA.back());
neighborsA.pop_back();
swap(*std::find(neighborsB.begin(), neighborsB.end(), a), neighborsB.back());
std::swap(*std::find(neighborsB.begin(), neighborsB.end(), a), neighborsB.back());
neighborsB.pop_back();

size_t edgeIndex;
for (size_t i = 0; i < m_edgeMap[a].size(); i++) {
edgeIndex = m_edgeMap[a][i];
const std::pair<size_t, size_t>& pair = m_edgePairs[edgeIndex];
if (pair.first == b || pair.second == b) {
swap(m_edgeMap[a][i], m_edgeMap[a].back());
std::swap(m_edgeMap[a][i], m_edgeMap[a].back());
m_edgeMap[a].pop_back();
break;
}
}

for (size_t i = 0; i < m_edgeMap[b].size(); i++) {
if (m_edgeMap[b][i] == edgeIndex) {
swap(m_edgeMap[b][i], m_edgeMap[b].back());
std::swap(m_edgeMap[b][i], m_edgeMap[b].back());
m_edgeMap[b].pop_back();
break;
}
}

swap(m_edgePairs[edgeIndex], m_edgePairs.back());
std::swap(m_edgePairs[edgeIndex], m_edgePairs.back());
m_edgePairs.pop_back();

size_t affectedIndex = m_edgePairs.size();
Expand Down Expand Up @@ -370,18 +362,15 @@ void Graph::removeEdges(size_t index)

void Graph::editEdgeInPlace(size_t edgeIndex, size_t a, size_t b)
{
// Allow ADL for swap
using std::swap;

auto& pair = m_edgePairs[edgeIndex];

// Remove references to the deleted edge from both endpoints.
for (size_t i = 0; i < m_edgeMap[pair.first].size(); i++) {
swap(m_edgeMap[pair.first][i], m_edgeMap[pair.first].back());
std::swap(m_edgeMap[pair.first][i], m_edgeMap[pair.first].back());
m_edgeMap[pair.first].pop_back();
}
for (size_t i = 0; i < m_edgeMap[pair.second].size(); i++) {
swap(m_edgeMap[pair.second][i], m_edgeMap[pair.second].back());
std::swap(m_edgeMap[pair.second][i], m_edgeMap[pair.second].back());
m_edgeMap[pair.second].pop_back();
}

Expand Down Expand Up @@ -430,7 +419,7 @@ void Graph::swapEdgeIndices(size_t edgeIndex1, size_t edgeIndex2)
*changeTo1[0] = edgeIndex1;
*changeTo1[1] = edgeIndex1;

swap(m_edgePairs[edgeIndex1], m_edgePairs[edgeIndex2]);
std::swap(m_edgePairs[edgeIndex1], m_edgePairs[edgeIndex2]);
}

size_t Graph::edgeCount() const
Expand Down
135 changes: 75 additions & 60 deletions avogadro/io/cmlformat.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -173,78 +173,93 @@ class CmlFormatPrivate
Atom atom;
while (node) {
// Step through all of the atom attributes and store them.
xml_attribute attribute = node.attribute("elementType");
if (attribute) {
unsigned char atomicNumber =
Elements::atomicNumberFromSymbol(attribute.value());
atom = molecule->addAtom(atomicNumber);
} else {
// There is no element data, this atom node is corrupt.
error += "Warning, corrupt element node found.";
return false;
}

attribute = node.attribute("id");
if (attribute)
atomIds[std::string(attribute.value())] = atom.index();
else // Atom nodes must have IDs - bail.
return false;

// Check for 3D geometry.
attribute = node.attribute("x3");
if (attribute) {
xml_attribute y3 = node.attribute("y3");
xml_attribute z3 = node.attribute("z3");
if (y3 && z3) {
// It looks like we have a valid 3D position.
Vector3 position(lexicalCast<double>(attribute.value()),
lexicalCast<double>(y3.value()),
lexicalCast<double>(z3.value()));
atom.setPosition3d(position);
{
/* Element Attribute Check */
xml_attribute elementAtt = node.attribute("elementType");
if (elementAtt) {
unsigned char atomicNumber =
Elements::atomicNumberFromSymbol(elementAtt.value());
atom = molecule->addAtom(atomicNumber);
} else {
// Corrupt 3D position supplied for atom.
return false;
}
} else if ((attribute == node.attribute("xFract"))) {
if (!molecule->unitCell()) {
error += "No unit cell defined. "
"Cannot interpret fractional coordinates.";
// There is no element data, this atom node is corrupt.
error += "Warning, corrupt element node found.";
return false;
}
xml_attribute& xF = attribute;
xml_attribute yF = node.attribute("yFract");
xml_attribute zF = node.attribute("zFract");
if (yF && zF) {
Vector3 coord(static_cast<Real>(xF.as_float()),
static_cast<Real>(yF.as_float()),
static_cast<Real>(zF.as_float()));
molecule->unitCell()->toCartesian(coord, coord);
atom.setPosition3d(coord);
} else {
error += "Missing y or z fractional coordinate on atom.";
}

{
/* ID Attribute Check */
xml_attribute idAtt = node.attribute("id");
if (idAtt)
atomIds[std::string(idAtt.value())] = atom.index();
else // Atom nodes must have IDs - bail.
return false;
}

// Check for 3D geometry.
{
/* XYZ3 and Fract Attribute Check */
xml_attribute x3Att = node.attribute("x3");
xml_attribute xFractAtt = node.attribute("xFract");
if (x3Att) {
xml_attribute y3 = node.attribute("y3");
xml_attribute z3 = node.attribute("z3");
if (y3 && z3) {
// It looks like we have a valid 3D position.
Vector3 position(lexicalCast<double>(x3Att.value()),
lexicalCast<double>(y3.value()),
lexicalCast<double>(z3.value()));
atom.setPosition3d(position);
} else {
// Corrupt 3D position supplied for atom.
return false;
}
} else if (xFractAtt) {
if (!molecule->unitCell()) {
error += "No unit cell defined. "
"Cannot interpret fractional coordinates.";
return false;
}
xml_attribute yF = node.attribute("yFract");
xml_attribute zF = node.attribute("zFract");
if (yF && zF) {
Vector3 coord(static_cast<Real>(xFractAtt.as_float()),
static_cast<Real>(yF.as_float()),
static_cast<Real>(zF.as_float()));
molecule->unitCell()->toCartesian(coord, coord);
atom.setPosition3d(coord);
} else {
error += "Missing y or z fractional coordinate on atom.";
return false;
}
}
}

// Check for 2D geometry.
attribute = node.attribute("x2");
if (attribute) {
xml_attribute y2 = node.attribute("y2");
if (y2) {
Vector2 position(lexicalCast<double>(attribute.value()),
lexicalCast<double>(y2.value()));
atom.setPosition2d(position);
} else {
// Corrupt 2D position supplied for atom.
return false;
{
/* XY2 Attribute Check */
xml_attribute x2Att = node.attribute("x2");
if (x2Att) {
xml_attribute y2 = node.attribute("y2");
if (y2) {
Vector2 position(lexicalCast<double>(x2Att.value()),
lexicalCast<double>(y2.value()));
atom.setPosition2d(position);
} else {
// Corrupt 2D position supplied for atom.
return false;
}
}
}

// Check formal charge.
attribute = node.attribute("formalCharge");
if (attribute) {
auto formalCharge = lexicalCast<signed int>(attribute.value());
atom.setFormalCharge(formalCharge);
{
/* Formal Charge Attribute Check */
xml_attribute formalChargeAtt = node.attribute("formalCharge");
if (formalChargeAtt) {
auto formalCharge = lexicalCast<signed int>(formalChargeAtt.value());
atom.setFormalCharge(formalCharge);
}
}

// Move on to the next atom node (if there is one).
Expand Down

1 comment on commit dd05561

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ERROR: clang-format-diff detected formatting issues. See the artifact for a patch or run clang-format on your branch.

Please sign in to comment.