Skip to content

Check and fix particle tag consistency in lammps #247

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
andeplane wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Check and fix particle tag consistency in lammps #247

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1eb49f3
Refactor: Use 'tag' instead of 'id' for atom extraction
cursoragent Nov 30, 2025
9ecd09e
feat: Add getTagPointer and use tag for particle IDs
cursoragent Nov 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions cpp/lammpsweb/lammpsweb.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -580,6 +580,17 @@ long LAMMPSWeb::getIdPointer() {
return reinterpret_cast<long>(ptr);
}

long LAMMPSWeb::getTagPointer() {
auto ptr = lammps_extract_atom((void *)m_lmp, "tag");

// Tag may not exist for all atom styles, return 0 if not available
if (!ptr) {
return 0;
Copy link
Owner Author

@andeplane andeplane Nov 30, 2025

Choose a reason for hiding this comment

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

@cursoragent should it be return null;?

Copy link

@cursor cursor bot Nov 30, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Create PR

No, return 0; is correct here! Here's why:

The function signature is:

long LAMMPSWeb::getTagPointer()

It returns a long (an integer type), not a pointer type. The function converts memory addresses to long values so they can be passed to JavaScript through the Emscripten interface.

In C++:

  • nullptr or NULL should be used for pointer types (void*, int*, etc.)
  • 0 should be used for integer types like long

Since we're returning a long that represents a memory address (or lack thereof), 0 is the appropriate sentinel value to indicate "no valid pointer available."

This matches the pattern used by the other pointer-returning functions in the codebase, and works correctly with the TypeScript check:

const tagPtr = input.lammps.getTagPointer();
const particleIdPtr = tagPtr !== 0 ? tagPtr / 4 : input.lammps.getIdPointer() / 4;

So the current implementation is correct! 👍

Open in Cursor Open in Web

Learn more about Cursor Agents

}

return reinterpret_cast<long>(ptr);
}

long LAMMPSWeb::getTypePointer() {
auto ptr = lammps_extract_atom((void *)m_lmp, "type");

Expand Down
2 changes: 2 additions & 0 deletions cpp/lammpsweb/lammpsweb.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ class LAMMPSWeb
long getBondsDistanceMapPointer();
long getPositionsPointer();
long getIdPointer();
long getTagPointer();
long getTypePointer();
long getBondsPosition1Pointer();
long getBondsPosition2Pointer();
Expand Down Expand Up @@ -129,6 +130,7 @@ EMSCRIPTEN_BINDINGS(LAMMPSWeb)
.function("getPositionsPointer", &LAMMPSWeb::getPositionsPointer)
.function("getBondsDistanceMapPointer", &LAMMPSWeb::getBondsDistanceMapPointer)
.function("getIdPointer", &LAMMPSWeb::getIdPointer)
.function("getTagPointer", &LAMMPSWeb::getTagPointer)
.function("getTypePointer", &LAMMPSWeb::getTypePointer)
.function("getCellMatrixPointer", &LAMMPSWeb::getCellMatrixPointer)
.function("getOrigoPointer", &LAMMPSWeb::getOrigoPointer)
Expand Down
11 changes: 8 additions & 3 deletions src/modifiers/syncparticlesmodifier.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,12 @@ class SyncParticlesModifier extends Modifier {

const positionsPtr = input.lammps.getPositionsPointer() / 4;
const typePtr = input.lammps.getTypePointer() / 4;
const idPtr = input.lammps.getIdPointer() / 4;

// Try to use tag first (persistent particle identifier), fallback to id if not available
const tagPtr = input.lammps.getTagPointer();
const particleIdPtr =
tagPtr !== 0 ? tagPtr / 4 : input.lammps.getIdPointer() / 4;

const positionsSubarray = input.wasm.HEAPF32.subarray(
positionsPtr,
positionsPtr + 3 * numParticles,
Expand All @@ -58,8 +63,8 @@ class SyncParticlesModifier extends Modifier {
typePtr + numParticles,
) as Int32Array;
const idSubarray = input.wasm.HEAP32.subarray(
idPtr,
idPtr + numParticles,
particleIdPtr,
particleIdPtr + numParticles,
) as Int32Array;

newParticles.positions.set(positionsSubarray);
Expand Down
1 change: 1 addition & 0 deletions src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export type LammpsWeb = {

getPositionsPointer: () => number;
getIdPointer: () => number;
getTagPointer: () => number;
getTypePointer: () => number;
getCellMatrixPointer: () => number;
getOrigoPointer: () => number;
Expand Down