Skip to content
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

"ERROR: Negative bin computed" for lattice vectors with negative component? #63

Closed
acroy opened this issue Apr 19, 2024 · 7 comments
Closed

Comments

@acroy
Copy link

acroy commented Apr 19, 2024

It seems there is an issue in the serial calculator when a component of one of the lattice vectors is negative. I took the first frame of the nonorth2 example in test_suite-lsq - basically graphene I guess. This works fine with the python example of the serial_chimes_calculator. Then I changed the cell vectors to have 120 degrees instead of 60 and remapped the atom positions into the new cell. Now the calculator fails with "ERROR: Negative bin computed". The files are attached.

As an aside remark, I think it is not good to call exit in the library code as there is no way to catch that in the program calling into the library.

params.txt
input-60.xyz.txt
input-120.xyz.txt

@nirgoldman
Copy link
Collaborator

nirgoldman commented Apr 19, 2024 via email

@acroy
Copy link
Author

acroy commented Apr 19, 2024

Nope. Same behavior on develop. (there doesn't seem to be a difference between main and develop?)

@acroy
Copy link
Author

acroy commented Apr 20, 2024

I think the problem is that xlo, ylo, zlo are not taken into account when calculating the bin indices:

bin_x_idx = floor( (sys_x[i] + extent_x * n_layers) / search_dist ) + 1;
bin_y_idx = floor( (sys_y[i] + extent_y * n_layers) / search_dist ) + 1;
bin_z_idx = floor( (sys_z[i] + extent_z * n_layers) / search_dist ) + 1;

(and a bit further down)

@nirgoldman
Copy link
Collaborator

Thanks for your attention to detail. It looks like the neighbor list correction did not get propagated to this repo after all. I should have a PR submitted by Monday.

@nirgoldman
Copy link
Collaborator

nirgoldman commented Apr 22, 2024

@acroy: I just submitted a hotfix PR for this issue (#64). I was able to run your test example and got agreement to the sixth decimal place for input-60.xyz and input-120.xyz for the energies and stress tensor components. Please test and let us know if there are additional issues.

@acroy
Copy link
Author

acroy commented Apr 23, 2024

Thanks for the quick response! I checked it and your PR fixes my problem. Maybe the input files could be part of the test to avoid regressions?

I haven't checked how the integration into LAMMPS or DFTB works, but does that mean that they have the same problem at the moment?

If you don't mind I will open another issue regarding the usage of "exit". I do think this is problematic (e.g. I was using chimes from a Jupyter notebook and the "negative bin error" killed the kernel).

@nirgoldman
Copy link
Collaborator

The same issue could exist with DFTB+ but isn't likely with LAMMPS. ChIMES/LAMMPS integration is achieved through the somewhat simpler chimesFF interface, which doesn't require passing cell lattice vectors to the ChIMES calculator.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants