Fix cell and virial transpose bug in dp_ipi #1353
Conversation
i-Pi assumes column-major storage to store cell and virial. Fix deepmodeling#1351. See i-Pi manual: https://ipi-code.org/i-pi/_static/manual.pdf See also i-pi/i-pi#48 and i-pi/i-pi#196.
njzjz
requested review from
amcadmus and
wanghan-iapcm
and removed request for
amcadmus
|
I am not sure I am correct - @wanghan-iapcm and @Yi-FanLi can you take a look? |
|
Besides, is #1123 related? |
Codecov Report
@@ Coverage Diff @@
## devel #1353 +/- ##
==========================================
- Coverage 75.56% 75.53% -0.03%
==========================================
Files 91 91
Lines 7500 7506 +6
==========================================
+ Hits 5667 5670 +3
- Misses 1833 1836 +3
Continue to review full report at Codecov.
|
source/ipi/driver.cc
Outdated
| @@ -158,7 +158,7 @@ int main(int argc, char * argv[]) | |||
| readbuffer_ (&socket, (char *)(cell_h), 9*sizeof(double)); | |||
| readbuffer_ (&socket, (char *)(cell_ih), 9*sizeof(double)); | |||
| for (int dd = 0; dd < 9; ++dd){ | |||
| dbox[dd] = cell_h[dd] * cvt_len; | |||
| dbox[dd] = cell_ih[dd] * cvt_len; | |||
There was a problem hiding this comment.
Have you checked what is cell_ih? does it the transpose of cell_h or the inverse of cell_h?
There was a problem hiding this comment.
Oh the manual said it is the inverse matrix... I didn't realize the difference between them.
The serversocket will then send a string “POSDATA”, then nine floats for the cell vectormatrix, then another nine floats for the inverse matrix.
(page 33)
|
For the system in #1351, the energy error before this PR is 0.02, and the error after this PR is 6e-6. |
|
The rest error should come from the unit conversion, I think. |
i-Pi assumes column-major storage to store cell and virial.
Fix #1351.
See i-Pi manual: https://ipi-code.org/i-pi/_static/manual.pdf (page 33 and page 78)
See also i-pi/i-pi#48 and i-pi/i-pi#196.