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

Support for DimeNet++ model #4427

Closed
arunppsg opened this issue Apr 7, 2022 · 2 comments · Fixed by #4432
Closed

Support for DimeNet++ model #4427

arunppsg opened this issue Apr 7, 2022 · 2 comments · Fixed by #4432
Labels

Comments

@arunppsg
Copy link
Contributor

arunppsg commented Apr 7, 2022

🚀 The feature, motivation and pitch

The DimeNet model is a model which can be used for molecular property prediction. DimeNet++ model is an upgrade to the DimeNet model. DimeNet++ model is faster than DimeNet and more accurate (ref). It will be nice to have DimeNet++ model in PyG.

There are two possible ways to add DimeNet++:

  1. The key difference between DimeNet and DimeNet++ is the InteractionBlock (DimeNet++ uses an updated version of InteractionBlock - called as InteractionPPBlock). Also, the authors proposed a minor upgrade to OutputBlock in DimeNet++ (OutputPPBlock). The forward method are same in both. Thus, we can subclass DimeNet++ to be a child class of DimeNet, and add an option in DimeNet __init__ method to use DimeNet++ architecture rather than DimeNet architecture (something like plusplus = True, then initialize plusplus model, thereby using OutputPPBlock and InteractionPPBlock, we also need to add a couple of additional parameters required by DimeNet++).
  2. Create an independent implementation of DimeNet++. There is a pytorch implementation here which I adapted for PyG. The adapted version can be found here.

Thanks for consideration! Let me know if I can provide any other additional details.

@rusty1s
Copy link
Member

rusty1s commented Apr 7, 2022

Yes, I always wanted to support DimeNet++ as well. Sub-classing sounds like the way to go for me. We also have an open PR regarding this #2421 but I hadn't have a deeper look yet.

Let me know if you think parts of this are re-usable. Otherwise, we can start from scratch :)

@arunppsg
Copy link
Contributor Author

arunppsg commented Apr 7, 2022

Oops, I didn't see the earlier issue. I too prefer subclassing. Here is my brief approach:

  • Let's upgrade the current OutputBlock to use the architecture proposed in DimeNet++. By this, DimeNet will also use OutputBlock presented in DimeNet++ block but I think this is fine since it is also beneficial DimeNet.
  • We will implement an new InteractionPPBlock
  • Add an user argument for plusplus and we can initialize variables accordingly

Also, we can add example and tests. I will put up a PR later today or by this week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants