-
Notifications
You must be signed in to change notification settings - Fork 62
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
Potential fails to predict structure without threebody interaction in threebody cutoff of 4 Å #64
Comments
I think the most robust way would be to skip the 3-body update to bond, i.e., treat it as a simple identity pass through, when there are no 3-body terms at all? |
Also see my previous PR, I have added a test case that just has one atom with no bond nor three body. The model runs fine |
@chc273 Thanks for the comments. m3gnet fails to run the "test_single_atoms" test function with the latest tensorflow but works with tensorflow=2.9.1. So it has to be this version of tensorflow. I don't know if it is of interest, but to make m3gnet work with the latest tensorflow, I'm trying to understand why the error in the snapshot emerged. Lines 288 to 305 in b2a92f7
From my understanding, the above code for basis_expansion tries to |
Please don't touch it. I added there for that reason you described |
Hi, I'm reporting that the current M3GNet Potential cannot predict structures without threebody interactions within the threebody cutoff of 4 Å. I believe this is not a desired behavior. Part of the error is listed here:
The error comes from the below lines:
m3gnet/m3gnet/models/_base.py
Lines 253 to 261 in b2a92f7
As designed,
self.model(graph)
actually works fine to get energy of structures without threebody interactions, but the line of@tf.function(experimental_relax_shapes=True)
leads to the bug. When commenting out these lines of@tf.function
from theget_energies
andget_efs_tensor
functions, this bug no longer exists.@chc273 When available, would you please comment on whether or not we should keep the
@tf.function
, and if we should keep them, how we can make thePotential
behave as designed?The text was updated successfully, but these errors were encountered: