-
Notifications
You must be signed in to change notification settings - Fork 10
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
Ideas for more test cases #12
Comments
Re: Ionic Transport I haven't been able to work with this paper's data, here's a summary of what I've run into: cif data is formatted as non-ASE compliant JSON, and Xtals.jl processing on a manual conversion is unable to convert to P1 space group. Full Details & Reproduction
I am not comfortable with space groups, so I have no idea where conversion is going wrong. Also, I do not believe ase specifies their exact JSON format. I've looked at the HTGW paper, and I am more confident in getting something working, I just need to wrap my head around the wavefunctions and formulas. I may just work off its parent database (C2DB). |
Wow, thanks for taking a crack at this! I visualized your manually converted CIF in VESTA, and I can't see any major red flags so I think the structure conversion is working fine. I'd have to spend a bit more time than I have right now to troubleshoot the conversion issues, but two questions:
|
manual = ase.io.read("LiNdP4O12_manual.cif") # I 1 2/c 1
type(manual) == ase.atoms.Atoms
ase.io.write("LiNdP4O12_ase.cif", manual) # P 1 This is a non-issue: ase uses the "updated" cif spec that uses the 'space_group' category instead of 'symmetry', However, ase produces a 4x unit cell version of this compound for some reason, Nd4 P16 O48 Li4 (due to P1 conversion?). I will need to test with more compounds. Both Xtals and AtomGraphs are able to read this in: Julia Outputjulia> c = Crystal("LiNdP4O12_ase.cif")
┌ Info: Crystal LiNdP4O12_ase.cif has space group. I am converting it to P1 symmetry.
└ To prevent this, pass `convert_to_p1=false` to the `Crystal` constructor.
Name: LiNdP4O12_ase.cif
Bravais unit cell of a crystal.
Unit cell angles α = 90.000000 deg. β = 90.100000 deg. γ = 90.000000 deg.
Unit cell dimensions a = 9.844000 Å. b = 7.008000 Å, c = 13.250000 Å
Volume of unit cell: 914.073072 ų
# atoms = 72
# charges = 0
chemical formula: Dict(:P => 4, :Li => 1, :Nd => 1, :O => 12)
space Group: P1
symmetry Operations:
'x, y, z'
bonding graph:
# vertices = 72
# edges = 0
julia> n = AtomGraph(c)
┌ Warning: Your cutoff radius is quite large relative to the size of your unit cell. This may cause issues with neighbor list generation, and will definitely cause a very dense graph. To avoid issues, I'm setting it to be approximately equal to the smallest unit cell dimension.
└ @ AtomGraphs ~/.julia/packages/AtomGraphs/TnX1D/src/graph_building.jl:164
AtomGraph{Crystal} with 72 nodes, 480 edges
atoms: ["Nd", "Nd", "Nd", "Nd", "P", "P", "P", "P", "P", "P", "P", "P", "P", "P", "P", "P", "P", "P", "P", "P", "O", "O", "O", "O", "O", "O", "O", "O", "O", "O", "O", "O", "O", "O", "O", "O", "O", "O", "O", "O", "O", "O", "O", "O", "O", "O", "O", "O", "O", "O", "O", "O", "O", "O", "O", "O", "O", "O", "O", "O", "O", "O", "O", "O", "O", "O", "O", "O", "Li", "Li", "Li", "Li"]
I was also wondering why space groups weren't being utilized here, as I feel they could be nice when dealing with crystal prototypes or anisotropic properties. But I guess dealing with P1 only is easier, and space group info should be embedded in the graph already. Also, I tried to change the manual cif to xyz, but Xtals can only read in cif and cssr. And oops, I forgot that crystals have to be symmetric... |
This multiplication of the unit cell is not really ideal, although in principle you're right that it shouldn't matter. At the moment, in practice, it does, because the pooling operations aren't smart enough, so you'd still get different results from a primitive cell graph vs. one of these supercelll-type graphs. All the more motivation for structurally aware pooling, cf #10...
They would definitely be nice, and I would love it if there were a
The first part is true, but I disagree with the second part...you do strictly lose information going from 3D coordinates to an adjacency matrix representation, e.g. you don't know anything about angles, which are crucial to symmetries...
Xtals can read xyz, you just need a different function because it reads to an |
Saving these here because a DM Slack thread with @venkvis is not the ideal archival solution, also in case others want to comment/discuss...
The text was updated successfully, but these errors were encountered: