fixed graph detach and embeddings in dataset_loss

coarse-graining / cgnet

learning coarse-grained force fields

BSD 3-Clause "New" or "Revised" License

57 stars 26 forks source link

fixed graph detach and embeddings in dataset_loss #121

Closed nec4 closed 5 years ago

nec4 commented 5 years ago

Development:

[x] Implement feature / fix bug
[ ] Add documentation
[x] Add tests

Checks:
[x] Run nosetests
[x] Check pep8 compliance

This PR fixes a bug in dataset_loss, where the embeddings were not forwarded properly, which thereby precluded its use for CGSchNet models. Additionally, the loss was not being detached during accumulation, and so memory issues were possible. I plan to add dataset loss test for a random CGSchNet architecture.

nec4 commented 5 years ago

Ok - added test_schnet_dataset_loss() to test_utils.py and ran autopep8. This new test just checks to see if dataset_loss works with CGSchNet models. All tests pass for me. Let me know if there is anything to fix/address or add.

nec4 commented 5 years ago

Thanks for the feedback. I have updated the comments/docs to address the points you have raised. Let me know if you think there is anything more to change or add.