search

SPARC-X / SPARC-X-API

GNU General Public License v3.0
11 stars 10 forks source link

Additional DFT calls during ASE-geometric optimization #20

Open alchem0x2A opened 1 year ago

alchem0x2A commented 1 year ago

make sure the sparc api only makes 1 dft call if multiple calls to energy / force / stresses are made

ltimmerman3 commented 1 year ago

Note that this isn't specific to optimization. If you load a saved aimd or other file and call atoms.get_potential_energy() followed by get_forces(), two DFT calls are made (even if specifying PRINT_FORCES). Similarly, calling get_forces() first results in the same behavior. I have verified that 'energy' shows up in the atoms.calc.results dictionary when calling get_forces(). There must be some breakage in the 'get_property' function or somewhere similar.

alchem0x2A commented 1 year ago

Finally some updates on this issue. It seems a more interwoven one than I previous thought.

There are several causes (at least what I discovered so far) that nullify the cached results. Basically when calling calc.get_property, it invokes calc.check_state(atoms) which compares if any properties like positions / cell have changed. The following can cause a non-empty return from check_state: 1) pbc state of input atoms different than that parsed from the SPARC files. Currently I intend to limit the code to only accept full PBC 2) init_magmom of atoms --> need to make sure both atoms to be checked and cached have 0 init_magmom if not specified 3) float point error. The default tol for check_state is 1e-15, which is unfortunately lower than the precision in some SPARC outputs (e.g. .static files have atomic position with 10 digits) --> overwrite the check_state function 4) When reading atomic position from .static / .geopt etc the position may be wrapped in the pbc. --> add an alternative check if either original atoms or wrapped atoms are identical to calc.atoms.positions

ajmedford commented 1 year ago

Sounds tricky.

A few thoughts:

The other fixes sound very reasonable.

From: T.Tian @.> Sent: Saturday, August 19, 2023 11:43 AM To: SPARC-X/sparc-dft-api @.> Cc: Subscribed @.***> Subject: Re: [SPARC-X/sparc-dft-api] Additional DFT calls during ASE-geometric optimization (Issue #20)

Finally some updates on this issue. It seems a more interwoven one than I previous thought.

There are several causes (at least what I discovered so far) that nullify the cached results. Basically when calling calc.get_property, it invokes calc.check_state(atoms) which compares if any properties like positions / cell have changed. The following can cause a non-empty return from check_state:

  1. pbc state of input atoms different than that parsed from the SPARC files. Currently I intend to limit the code to only accept full PBC
  2. init_magmom of atoms --> need to make sure both atoms to be checked and cached have 0 init_magmom if not specified
  3. float point error. The default tol for check_state is 1e-15, which is unfortunately lower than the precision in some SPARC outputs (e.g. .static files have atomic position with 10 digits) --> overwrite the check_state function
  4. When reading atomic position from .static / .geopt etc the position may be wrapped in the pbc. --> add an alternative check if either original atoms or wrapped atoms are identical to calc.atoms.positions

— Reply to this email directly, view it on GitHubhttps://github.com/SPARC-X/sparc-dft-api/issues/20#issuecomment-1685031184, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ABJRPEVH26SJPV6BLRJFV2LXWDNJ5ANCNFSM6AAAAAAZSTCDOU. You are receiving this because you are subscribed to this thread.Message ID: @.***>

alchem0x2A commented 1 year ago

sounds good @ajmedford for now let's still keep this issue open as it may involve opening another 1~2 issues regarding BC and position wrapping.

@ltimmerman3 #31 was a quick fix for this issue that I tested working on the 2 examples within the examples directory and unit test, however please let me know if it can fix issues regarding your existing code examples (if full PBCs are used). If so I intend to further make the solution more flexible, e.g. to support more BC writing and reading.

ajmedford commented 1 year ago

Sounds good, thanks Tian!

From: T.Tian @.> Sent: Saturday, August 19, 2023 12:38 PM To: SPARC-X/sparc-dft-api @.> Cc: Medford, Andrew J @.>; Mention @.> Subject: Re: [SPARC-X/sparc-dft-api] Additional DFT calls during ASE-geometric optimization (Issue #20)

sounds good @ajmedfordhttps://github.com/ajmedford for now let's still keep this issue open as it may involve opening another 1~2 issues regarding BC and position wrapping.

@ltimmerman3https://github.com/ltimmerman3 #31https://github.com/SPARC-X/sparc-dft-api/pull/31 was a quick fix for this issue that I tested working on the 2 examples within the examples directory and unit test, however please let me know if it can fix issues regarding your existing code examples (if full PBCs are used). If so I intend to further make the solution more flexible, e.g. to support more BC writing and reading.

— Reply to this email directly, view it on GitHubhttps://github.com/SPARC-X/sparc-dft-api/issues/20#issuecomment-1685041457, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ABJRPEQKOVU5E56T7ZDXAMTXWDTZXANCNFSM6AAAAAAZSTCDOU. You are receiving this because you were mentioned.Message ID: @.***>

ltimmerman3 commented 1 year ago

Another thought on this: until the socket is active, the most efficient way to run optimizations is to use the SPARC built-in optimizers. I think we should make it very clear that the default way to run optimizations should be to let SPARC handle everything. This avoids the issue with additional DFT calls and the overhead associated with initiating a new DFT calculation from scratch every iteration. If you'd like, I can continue picking at this issue if it would allow you to focus more on the socket. As @ajmedford mentioned, it will be important to be able to use both periodic and dirichlet bc's since this is a major advantage of SPARC.