-
Notifications
You must be signed in to change notification settings - Fork 71
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
[WIP] Development branch with many body feature and energy mapping #162
Conversation
nw13slx
commented
Apr 8, 2020
•
edited by YuuuXie
Loading
edited by YuuuXie
- Energy mapping, and enhanced the ase interface @YuuuuXie
- Manybody kernel by @AldoGl @ClaudioZeni
- much shorter unit tests
- easier gp initialization
- more structured interface for different kernels
- compatibility with ZAPPA @stevetorr
- Faster GP by using copy-on-right with global variable for multiprocessing. (For 188 atomic environments and mc_sephyps kernels, it takes about 0.83 second to predict forces for atomic environment, and 5.7 seconds for set_L_alpha() with 32 Skylake cores. )
…in/share_memory Conflicts: flare/ase/otf_md.py flare/dft_interface/vasp_util.py tests/test_ase_setup/test_otf.py
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I reviewed the many-body part and here are a few possible enhancement which could be done in the future, will probably do it in a separate branch.
-
env.py
get_2_body_arrays
andget_2_body_arrays_ind
are redundant. The latter is used in theget_m_body_arrays
, and could be changed into the similar style asget_3_body_arrays
, i.e. search arrays from a shorter list (but we need to presume that 2-body cutoff is larger than many body) -
kernels: sc.py, mc_simple.py, mc_sephyp.py
For many body kernel, we’d better store the coordination number for the training data directly instead of calculating it from the neighbor list every time in the kernel function -
test_kernels.py, test_mc_mb_kernel.py
Simplify the code, and mergetest_mc_mb.kernel.py
intotest_mc_kernels.py
-
mgp.py, mgp_en.py
Just think of this, since most of the code is duplicated, I will consider merging the two, and set up option for “energy map” or “force map”.
Also, remove theotf.py
in mgp folder, since (1) it is lack of maintenance, (2) it can be substituted by ase/otf.py completely. So it is useless now
@@ -190,6 +200,97 @@ class to allow for njit acceleration with Numba. | |||
return bond_array_2, bond_positions_2, etypes | |||
|
|||
|
|||
@njit | |||
def get_2_body_arrays_ind(positions, atom: int, cell, cutoff_2: float, species): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new changes will be made in Lixin's branch of separating cutoffs
flare/env.py
Outdated
""" | ||
# TODO: this can be probably improved using stored arrays, redundant calls to get_2_body_arrays | ||
# Get distances, positions, species and indexes of neighbouring atoms | ||
bond_array_mb, __, _, bond_inds = get_2_body_arrays_ind( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the atomic environment, the coordination number of each neighbor atom is calculated. Then if we loop over atoms in a Structure, then the coordination number of the same atom will be calculated multiple times, which will affect the efficiency.
Let's consider if we can add a method to calculate coordination number for each atom in the whole structure, so as to avoid the redundant computation.
return kern | ||
|
||
|
||
def many_body_mc_jit(bond_array_1, bond_array_2, neigh_dists_1, neigh_dists_2, num_neigh_1, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We might store the coordination number for the training data directly instead of calculating it every time
tests/test_kernels.py
Outdated
|
||
|
||
def test_many_body_force(): | ||
"""Check that the analytical force kernel matches finite difference of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This chunk of code can be simplified via loops....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this will be address in pull request #166
tests/test_mc_mb_kernel.py
Outdated
# ----------------------------------------------------------------------------- | ||
|
||
|
||
def test_many_body_force(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as test_kernels.py, can be simplified
@@ -0,0 +1,221 @@ | |||
import numpy as np |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mgp_en.py and mgp.py are quite duplicated. I'll consider merging them, and set up options for "energy map" or "force map"
fix manybody kernel etype issue