Skip to content
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

Add support for charged molecules in TensorNet (total charge) #238

Merged
merged 9 commits into from
Nov 3, 2023

Conversation

guillemsimeon
Copy link
Collaborator

Training using total charges should be possible by just setting the charge hyperparameter to True in the yaml file.

@RaulPPelaez
Copy link
Collaborator

There are some error:

FAILED tests/test_model.py::test_torchscript[cpu-tensornet] - RuntimeError: 
Variable 'q' previously had type Optional[Tensor] but is now being assigned to a value of type int
:
  File "/usr/share/miniconda3/envs/torchmd-net/lib/python3.10/site-packages/torchmdnet/models/tensornet.py", line 221
            # I trick the model into thinking that the masked edges pertain to the extra atom
            if q is None:
                q = 0
                ~ <--- HERE
            else:
                batchp = torch.cat((batch, batch[-1] + 1), dim=0)

Default q to something like

q = torch.zeros(1, device=z.device, dtype=pos.dtype)

@RaulPPelaez
Copy link
Collaborator

Looks good and GPU tests pass. Lets merge. Thanks @guillemsimeon !

@RaulPPelaez RaulPPelaez merged commit af51c58 into torchmd:main Nov 3, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants