allow structures without three-body interaction in cutoff radius -- isolated atoms or two-body regions

[JiQi535]

The previous code is not working for structures containing isolated atoms or only two-body interactions in certain 5 Å spherical regions.

This PR solve this problem without much impact on computation speed. It has been tested to work on a few cases:

1. Getting a constant energy for structures having one isolated atom but different lattice parameters (e.g., 6, 7, 8, 9, all above cutoff radius of 5 Å)
2. Predicting structures with only two-body interactions.
3. Predicting structures with both two-body and three-body regions.

Some re-formattings are not related to "allow structures without three-body interaction in cutoff radius", as they were done by Black.

Finally, thanks @kenko911 for providing suggestions and helpful discussions, though the changes I have done are not the same with yours. Please kindly have a look and point out if there is unseen problem in this way of solving the issue. Thanks and looking forward to discussing with you.

[shyuep]

I am trying to figure out the code, but in general, I am not in favor of solutions that depend on the ext packages treating isolated atoms as special cases. Isolated atom support should be part and parcel of the model and layers. In other words, given a graph of isolated atoms, the model and layers should yield a result.

[JiQi535]

I am trying to figure out the code, but in general, I am not in favor of solutions that depend on the ext packages treating isolated atoms as special cases. Isolated atom support should be part and parcel of the model and layers. In other words, given a graph of isolated atoms, the model and layers should yield a result.

Thanks for the comments. I agree with it. I think there is overlapping in the ase.py and pymatgen.py ext package scripts about processing the structures or atoms into graph. Shall we unify that part into a function file in utils? Or do you have any suggestions where this piece of code should go? @shyuep

[kenko911]

@JiQi535 Thanks a lot for your contributions! I just have a look on the changes and I think the solution is not ideal.

1. It seems to me that you try adding artificial edges for the case of an isolated atom to construct a dgl graph. In this way, the case of isolated atom will have message passing, which is not working as expected.
2. I think compute_3body doesn't not need to be modified. We can set up different cases in create_line_graph so that we don't need to go through compute_3body everytime.

[JiQi535]

1. It seems to me that you try adding artificial edges for the case of an isolated atom to construct a dgl graph. In this way, the case of isolated atom will have message passing, which is not working as expected.

I created minimum possible number of artificial edges longer than cutoff so that node feature about atom type can be stored. Passing an empty graph seems not able to distinguish different isolated atoms. These artificial edges have bond length over cutoff radius, so that their respective rbf values equals to zero (the same as the rbf right at cutoff radius). Therefore, these edges do pass through convolutions, but the results won't be affected by these artificial bonds (edges), as they are treated same as bond equal to cutoff radius. @kenko911

2. I think compute_3body doesn't not need to be modified. We can set up different cases in create_line_graph so that we don't need to go through compute_3body everytime.

The compute_3body is slightly modified to make sure that sbf equals to 0 for bonds over cutoff radius, whereas it was previously only equal to 0 when bond is equal to cutoff radius. I think this change is physical.

[kenko911]

1. There is a more efficient way to storage the node type without adding artificial edges. Moreover, in this case you will have extra nodes and the total energy would be not be equal to the energy of an isolated atom.
2. I think the case of an isolated atom and with only two-body should not pass through comput_3body.

[JiQi535]

1. There is a more efficient way to storage the node type without adding artificial edges. Moreover, in this case you will have extra nodes and the total energy would be not be equal to the energy of an isolated atom.

From my tests, the energy increases smoothly when bond distances increase until bond distances equal to cutoff radius, after which the energy remains constant and equals to energy when bond lengths equal to 5 angstrom. As the energy of structure without isolated atoms is not changed after my changes, and isolated atoms have energy same as the energy when bond lengths equal to 5 angstrom, I don't think the energy of isolated atom is wrong. Could you show me an example that isolated atom has wrong energy?

[JiQi535]

I am trying to figure out the code, but in general, I am not in favor of solutions that depend on the ext packages treating isolated atoms as special cases. Isolated atom support should be part and parcel of the model and layers. In other words, given a graph of isolated atoms, the model and layers should yield a result.

@shyuep In the latest commit, I have moved the overlapped codes of graph construction in ext packages related .py files to matgl/graph/converters.py. Please have a look.

Again, I believe that the current implementation handles the special cases, i.e., (1) isolated atoms and (2) regions with only two-body interactions correctly without the extra effort to batch extra graph with three-body interactions (what the TF version needs). I have sent my test structures to @kenko911, and I'm looking forward to any new discussions or suggestions.

[kenko911]

@JiQi535 Could you please also test the training with isolated atoms and two-bodys? Thanks!

[JiQi535]

@JiQi535 Could you please also test the training with isolated atoms and two-bodys? Thanks!

I have made some further improvements so that training with even all isolated atoms or structures with two-body interactions will work properly. I will close this PR, and we can move to this new PR #92