Support of all lammps dump coordinate style #179
Conversation
Codecov Report
@@ Coverage Diff @@
## devel #179 +/- ##
==========================================
+ Coverage 80.07% 81.25% +1.17%
==========================================
Files 54 54
Lines 4553 4550 -3
==========================================
+ Hits 3646 3697 +51
+ Misses 907 853 -54
Continue to review full report at Codecov.
|
|
|
UTs for "scaled" and "scaled & unfolded" have been added. |
dpdata/lammps/dump.py
Outdated
| if sf: | ||
| posis = (posis%1.)@cell # convert xsu to xs first | ||
| else: | ||
| posis = (posis - orig)%np.linalg.norm(cell,axis=1) |
There was a problem hiding this comment.
%np.linalg.norm(cell,axis=1) would not work for triclinic cells.
There was a problem hiding this comment.
Fixed. It is %cell.sum(axis=0) now.
dpdata/lammps/dump.py
Outdated
| @@ -83,7 +83,7 @@ def safe_get_posi(lines,cell,orig=np.zeros(3)) : | |||
| if sf: | |||
| posis = (posis%1.)@cell # convert xsu to xs first | |||
| else: | |||
| posis = (posis - orig)%np.linalg.norm(cell,axis=1) | |||
| posis = (posis - orig)%cell.sum(axis=0) | |||
There was a problem hiding this comment.
The 0th dim of cell is the frame. cell.sum(axis=0) sums the cell tensors of all frames. It doesn't make sense to me.
There was a problem hiding this comment.
The safe_get_posi function handles one frame each time. When the safe_get_posi function is called from system_data, the cell argument passed to safe_get_posi is returned from dumpbox2box function, which is always a 3*3 array.
There was a problem hiding this comment.
I see that cell is of shape [3,3], but%cell.sum(axis=0) would not give the right wrapped coordinated in a triclinic cell.
The correct way would be:
- convert the physical coordinates to scaled coordinates. Notice that scale cell is a cubic
- wrap the scaled coordinate by
posis%1. - convert the scaled coordinates back to the physical coordinates.
There was a problem hiding this comment.
It seems that [x,y,z]@inv(cell) can accurately scale the physical coordinates, where inv is the inverse operation.
| if not sf: | ||
| posis = (posis-orig)@np.linalg.inv(cell)# Convert to scaled coordinates for unscaled coordinates | ||
| return (posis%1)@cell # Convert scaled coordinates back to Cartesien coordinates |
There was a problem hiding this comment.
@y1xiaoc The scaled coordinates are converted to Cartesian coordinates, is it the expected behavior?
There was a problem hiding this comment.
For orig equals to zero this should be correct.
For a Cartesian and folded coordinates (with keys x, y and z, i.e. the originally supported one) with a non-zero orig, this will change the behavior by forcing the shifted coordinate fit into the cell. For example, for a coord orginally range in (0,10) and a orig = 5, original version will lead to a coord range in (-5,5) but current version (due to the %1 operation) will warp the coord back into range (0, 10) again.
I'm not sure what is the orig parameter used for (and it seems to be ignored for scaled coordinates in both original and current version) so I cannot tell this change of behavior is problematic or not.
Corresponding tests have been added now.