-
Notifications
You must be signed in to change notification settings - Fork 6
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
Missing PBC handled badly? #54
Comments
Versions:
|
Hm, what default would you want in this case?
I think if a lattice is provided, the extxyz spec says PBC is assumed.
…On Sat, 14 Sep 2024 at 14:26, Christoph Ortner ***@***.***> wrote:
using ExtXYZ
download("https://www.dropbox.com/scl/fi/z6lvcpx3djp775zenz032/Si-PRX-2018.xyz?rlkey=ja5e9z99c3ta1ugra5ayq5lcv&st=cs6g7vbu&dl=1",
"Si_dataset.xyz");
Si_dataset = ExtXYZ.load("Si_dataset.xyz");
results in
┌ Warning: 'pbc' not contained in dict. Defaulting to all-periodic boundary.
└ @ ExtXYZ ~/.julia/packages/ExtXYZ/dBeGN/src/atoms.jl:152
which refers to
Si_dataset[1].system_data.periodicity
# (true, true, true)
CC @wcwitt <https://github.com/wcwitt>
—
Reply to this email directly, view it on GitHub
<#54>, or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACXHHTQ3JB4QEYULAVWYQC3ZWR5ULAVCNFSM6AAAAABOHB35QOVHI2DSMVQWIX3LMV43ASLTON2WKOZSGUZDMNJVGMYTKNA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
I see - I shouldbhave checked that. So the dataset is problematic not the loader? |
I agree with Chuck that current behaviour matches spec at https://github.com/libAtoms/extxyz. If lattice is missing then we default to open boundaries, but if it's present the pbc is the correct default. |
Ok - I'll just have to manually fix the dataset then. For now I've just removed that system which I think is the correct thing anyhow. |
Thanks for the quick response. |
results in
which refers to
CC @wcwitt
The text was updated successfully, but these errors were encountered: