BUG: Write LAMMPS File crashes when exporting fewer than 1000 atoms

Cerrado Apto para principiantes
#1,818 0 comentarios 0 reacciones 0 asignados Ver en GitHub

Los mantenedores suelen responder en 3 días

@imikejackson ya está trabajando en esto.

Desde el 31/10/2026.

  • #1900 de @imikejackson — abierto

Evaluación

Dificultad
2/5
Tiempo estimado
1-3 horas
Aptitud para principiantes
80/100
Tipo de issue
Error
Claridad
Bien especificado
Estado de actividad
Activo
Stack tecnológico
cpp
Área
data

Línea de trabajo

La división entre cero ocurre en WriteLAMMPSFile::operator() en src/Plugins/SimplnxCore/src/SimplnxCore/Filters/Algorithms/WriteLAMMPSFile.cpp, donde el incremento de progreso es numVerts / 1000 y luego se usa en i % increment. Protege el incremento para que sea al menos uno, o elimina la condición del módulo, y después añade el caso de ocho átomos en src/Plugins/SimplnxCore/test/WriteLAMMPSFileTest.cpp. Está terminado cuando esa prueba pasa y las entradas de 1, 999 y 1000 átomos se exportan correctamente.

Escrito por el modelo de indexación a partir del texto del issue.

Descripción

bug

Problem

WriteLAMMPSFileFilter accepts a valid eight-atom vertex geometry in preflight, then crashes during execution with an integer divide-by-zero error. It should write a LAMMPS data file containing eight atom records.

This was reproduced against simplnx commit fe6c4a15e5a5bffef2523f52cec01be33b1e62a9. The same algorithm and filter source are unchanged at 84c66066a26b9e098ab58748dc2f8e7bee8b01e9.

Steps to Reproduce

  1. Create a VertexGeom with the eight corners of a unit cube: (0,0,0), (1,0,0), (0,1,0), (1,1,0), (0,0,1), (1,0,1), (0,1,1), and (1,1,1).
  2. Create an int32 atom-label array with eight tuples, one component, and every value set to 1.
  3. Set Input Vertex Geometry and Atom Feature Labels to those objects, and set Output File Path to a writable path.
  4. Preflight succeeds without warnings. Execute the filter.
  5. The process terminates with a divide-by-zero error before it writes the first atom record.

The unit test below constructs these inputs directly; it needs no downloaded fixture.

Cause

WriteLAMMPSFile::operator() calculates the progress interval by integer division and then uses it as the divisor in a remainder operation:

usize numVerts = verts.getNumberOfTuples();
usize increment = numVerts / 1000;
for(usize i = 0; i < numVerts; i++)
{
  if(m_ShouldCancel)
  {
    return {};
  }
  if(i % increment == 0)
  {
    progressThrottle.updatePercent("Exporting Data", i, numVerts);
  }

For the eight-atom input, increment is zero and the first iteration evaluates 0 % 0. The same source condition applies to every nonempty input containing fewer than 1,000 atoms; the executed reproduction used eight atoms. The filter calls this algorithm before committing its atomic output file.

Verification Unit Test

Add this case to src/Plugins/SimplnxCore/test/WriteLAMMPSFileTest.cpp. It asserts successful execution and the expected eight atom records, labels, coordinates, and image flags.

#include "SimplnxCore/Filters/WriteLAMMPSFileFilter.hpp"

#include "simplnx/DataStructure/AttributeMatrix.hpp"
#include "simplnx/DataStructure/DataArray.hpp"
#include "simplnx/DataStructure/Geometry/VertexGeom.hpp"
#include "simplnx/Parameters/FileSystemPathParameter.hpp"
#include "simplnx/UnitTest/UnitTestCommon.hpp"

#include <catch2/catch.hpp>

#include <array>
#include <filesystem>
#include <fstream>
#include <iostream>
#include <sstream>
#include <string>

using namespace nx::core;

TEST_CASE("SimplnxCore::WriteLAMMPSFileFilter: Export eight atoms", "[SimplnxCore][WriteLAMMPSFileFilter][LAMMPSSmallVertexCount]")
{
  UnitTest::LoadPlugins();
  constexpr usize k_NumAtoms = 8;
  constexpr std::array<std::array<float32, 3>, k_NumAtoms> k_Positions = {
      {{0.0f, 0.0f, 0.0f}, {1.0f, 0.0f, 0.0f}, {0.0f, 1.0f, 0.0f}, {1.0f, 1.0f, 0.0f}, {0.0f, 0.0f, 1.0f}, {1.0f, 0.0f, 1.0f}, {0.0f, 1.0f, 1.0f}, {1.0f, 1.0f, 1.0f}}};
  const DataPath geometryPath({"Atoms"});
  const DataPath atomLabelsPath({"Atoms", "Vertex Data", "Atom Labels"});
  DataStructure dataStructure;
  auto* geometry = VertexGeom::Create(dataStructure, "Atoms");
  REQUIRE(geometry != nullptr);
  auto* vertices = Float32Array::CreateWithStore<Float32DataStore>(dataStructure, VertexGeom::k_SharedVertexListName, {k_NumAtoms}, {3}, geometry->getId());
  REQUIRE(vertices != nullptr);
  geometry->setVertices(*vertices);
  for(usize atom = 0; atom < k_NumAtoms; atom++)
  {
    for(usize component = 0; component < 3; component++)
    {
      (*vertices)[3 * atom + component] = k_Positions[atom][component];
    }
  }
  auto* vertexData = AttributeMatrix::Create(dataStructure, "Vertex Data", {k_NumAtoms}, geometry->getId());
  REQUIRE(vertexData != nullptr);
  geometry->setVertexAttributeMatrix(*vertexData);
  auto* labels = Int32Array::CreateWithStore<Int32DataStore>(dataStructure, "Atom Labels", {k_NumAtoms}, {1}, vertexData->getId());
  REQUIRE(labels != nullptr);
  labels->fill(1);
  UnitTest::CheckArraysInheritTupleDims(dataStructure);

  const auto outputPath = std::filesystem::current_path() / "lammps_eight_atoms.txt";
  std::filesystem::remove(outputPath);
  WriteLAMMPSFileFilter filter;
  auto args = filter.getDefaultArguments();
  args.insertOrAssign(WriteLAMMPSFileFilter::k_OutputFile_Key, std::make_any<FileSystemPathParameter::ValueType>(outputPath));
  args.insertOrAssign(WriteLAMMPSFileFilter::k_VertexGeomPath, std::make_any<DataPath>(geometryPath));
  args.insertOrAssign(WriteLAMMPSFileFilter::k_AtomLabelsPath_Key, std::make_any<DataPath>(atomLabelsPath));

  const auto preflight = filter.preflight(dataStructure, args);
  SIMPLNX_RESULT_REQUIRE_VALID(preflight.outputActions);
  REQUIRE(preflight.outputActions.warnings().empty());
  std::cout << "Preflight succeeded for eight vertices and eight int32 labels." << std::endl;
  const auto execution = filter.execute(dataStructure, args);
  SIMPLNX_RESULT_REQUIRE_VALID(execution.result);

  REQUIRE(std::filesystem::exists(outputPath));
  std::ifstream stream(outputPath);
  REQUIRE(stream.is_open());
  bool foundAtomCount = false;
  bool foundAtomSection = false;
  usize atomRecords = 0;
  for(std::string line; std::getline(stream, line);)
  {
    if(line == "8 atoms")
    {
      foundAtomCount = true;
    }
    if(line == "Atoms")
    {
      foundAtomSection = true;
      continue;
    }
    if(!foundAtomSection || line.empty())
    {
      continue;
    }
    REQUIRE(atomRecords < k_NumAtoms);
    std::istringstream row(line);
    usize atomId = 0;
    int32 atomType = 0;
    std::array<float32, 3> position{};
    std::array<int32, 3> imageFlags{};
    REQUIRE(static_cast<bool>(row >> atomId >> atomType >> position[0] >> position[1] >> position[2] >> imageFlags[0] >> imageFlags[1] >> imageFlags[2]));
    CHECK(atomId == atomRecords + 1);
    CHECK(atomType == 1);
    CHECK(position == k_Positions[atomRecords]);
    const std::array<int32, 3> expectedImageFlags = {0, 0, 0};
    CHECK(imageFlags == expectedImageFlags);
    atomRecords++;
  }
  CHECK(foundAtomCount);
  CHECK(foundAtomSection);
  CHECK(atomRecords == k_NumAtoms);
}

The complete test above was compiled in a standalone Catch2 target, linked to the unmodified Release simplnx and SimplnxCore libraries built from the reported commit. The runtime DLL and plugin hashes matched the source runtime. The standalone target registers one CTest case named LAMMPSSmallVertexCount.

Run only that registered reproduction:

cmake --build <reproduction-build> --config Release
ctest --test-dir <reproduction-build> -C Release -j 1 -R "^LAMMPSSmallVertexCount$" --output-on-failure --timeout 30

After adding the case to the repository test file and building the normal SimplnxCore test target, select it through the repository's Catch2 CTest registration:

ctest --test-dir <simplnx-build> -C Release -j 1 -R "WriteLAMMPSFileFilter: Export eight atoms" --output-on-failure

Observed failure:

Start 1: LAMMPSSmallVertexCount
1/1 Test #1: LAMMPSSmallVertexCount ...........***Exception: Numerical  0.03 sec
Preflight succeeded for eight vertices and eight int32 labels.
failed: fatal error condition with message: 'Divide by zero error'
Failed 1 test case, failed 1 assertion.

0% tests passed, 1 tests failed out of 1

CTest returned exit code 8. The test compiled successfully; this failure occurred during execution after successful preflight.

Proposed Fix

In WriteLAMMPSFile::operator(), ensure the progress interval is at least one before evaluating i % increment, or remove this extra modulo gate and let ThrottledMessageHandler control progress-message frequency. Keep the small-input regression test and cover the 1, 999, and 1,000 atom boundaries when implementing the fix.

No production fix was applied or tested for this report.

Notes

  • Reproduction environment: Windows x64, Visual Studio 2022, MSVC 19.44.35229, Release configuration.
  • This verifies the file export path. It does not run a LAMMPS simulation or validate simulation setup.
Lenguaje dominante
C++
Estrellas
18
Forks
15
Merge medio
3 d 15 h
PR fusionados (30 d)
27

Preparar el entorno

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Más de BlueQuartzSoftware/simplnx

Todos los issues de BlueQuartzSoftware/simplnx

Issues similares

Más issues de C++

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.