libAtoms / ExtXYZ.jl

Extended XYZ read/write support for Julia
MIT License
13 stars 6 forks source link

Incorrect file to dict transformation with multiple atoms #8

Closed jamesgardner1421 closed 3 years ago

jamesgardner1421 commented 3 years ago

Hi again I think I've found another issue.

When you add a second atom to the test file:

2
Lattice="20.0 0.0 0.0 0.0 20.0 0.0 0.0 0.0 20.0" Properties=species:S:1:pos:R:3:map_shift:I:3:n_neighb:I:1:gap_force:R:3:dft_force:R:3 config_type=isolated_atom gap_energy=-157.7272532 gap_virial="0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0" dft_energy=-158.54496821 cutoff=5.5 nneightol=1.2 pbc="T T T"
Si      1.00000000      2.00000000      3.00000000       -1       -1       -1        0       0.00000000       0.00000000       0.00000000       0.00000000       0.00000000       0.00000000
Si      4.00000000      5.00000000      6.00000000       -1       -1       -1        0       0.00000000       0.00000000       0.00000000       0.00000000       0.00000000       0.00000000

and then read the positions:

data = read_frame("test.xyz")
data["arrays"]["pos"]

3×2 Matrix{Float64}:
 1.0  2.0
 3.0  4.0
 5.0  6.0

The positions are in read in the incorrect order. You can actually also see this in the README example, the data is in the wrong order. When you write the file again it preserves the original structure so the transformation is correctly reversible but the dict is wrong.

I had a go at trying to fix this but it's a bit complicated for me. It arises due to the row major vs col major data representations in C and Julia but when you fix it for positions it will break something else in the info field since all the data is using the same convert function. Maybe you can find a nice way to fix it.

jameskermode commented 3 years ago

Funny you mention this, I just found the same issue. Fix coming shortly…

jameskermode commented 3 years ago

Should now be fixed. I'll do another patch release after a bit more testing.

jameskermode commented 3 years ago

Seems to work so I'm tagging a patch release now. Thanks for reporting, and do re-open if there's still a problem.