iRASPA / RASPA2

Classical molecular simulation code
Other
129 stars 83 forks source link

Set BoundaryCondition to CUBIC when possible #38

Closed Liozou closed 1 year ago

Liozou commented 2 years ago

Hi! I noticed that doing a simple MC simulation of faujasite resulted in this curious note in the output:

System Properties
===========================================================================
Unit cell size: 24.257600 24.257600 24.257600
Cell angles (radians)  alpha: 1.570796 beta: 1.570796 gamma: 1.570796
Cell angles (degrees)  alpha: 90.000000 beta: 90.000000 gamma: 90.000000
Number of unitcells [a]: 1
Number of unitcells [b]: 1
Number of unitcells [c]: 1

TRICLINIC Boundary conditions: alpha!=90 or beta!=90 or gamma!=90

so I investigated and it turns out that, in framework.c, lines 2001 and 3441 wrongly set the BoundaryCondition to TRICLINIC instead of CUBIC (and lines 3320 always overwrites the preceding lines into TRICLINIC as well).

This PR fixes that and also adds CUBIC as an option whenever only RECTANGULAR and TRICLINIC exist (by defaulting to RECTANGULAR). I believe there is actually no difference between CUBIC and RECTANGULAR in the current code, but at least this code allows specializing on CUBIC in future developments.

fxcoudert commented 1 year ago

Just for the record: the bug fixed by Fix Buckingham potentials in grids (https://github.com/iRASPA/RASPA2/pull/38/commits/c5719dffc26799b5e4341b2b48acc9dc7b780ad5) was really crippling for us, until we were given the fix by David Sholl's group. Maybe it could be merged into RASPA for all users who aren't aware of it?