-
-
Notifications
You must be signed in to change notification settings - Fork 10
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.
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
base: main
Are you sure you want to change the base?
Check and fix particle tag consistency in lammps #247
Conversation
Co-authored-by: andershaf <andershaf@gmail.com>
|
Cursor Agent can help with this pull request. Just |
|
@cursoragent this is terrible. It does not always exist, and should DEFINITELY not replace the getId pointer function. |
|
You're absolutely right, and I apologize for the mistake! I've now implemented the correct solution that: Changes Made1. Added a new
2. Updated particle tracking logic in
Key Points✅ The solution now properly checks if Learn more about Cursor Agents |
Co-authored-by: andershaf <andershaf@gmail.com>
|
|
||
| // Tag may not exist for all atom styles, return 0 if not available | ||
| if (!ptr) { | ||
| return 0; |
There was a problem hiding this comment.
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;?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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++:
nullptrorNULLshould be used for pointer types (void*,int*, etc.)0should be used for integer types likelong
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! 👍
Learn more about Cursor Agents
Use LAMMPS "tag" field for particle identification instead of "id" to ensure consistent tracking across simulation steps.
The "tag" field in LAMMPS provides a persistent unique identifier for particles, which is crucial for reliable tracking over time. The "id" field, while also an identifier, may not be as consistent for this purpose.