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 MoleculeGPT #9710

Open
wants to merge 39 commits into
base: master
Choose a base branch
from
Open

Add MoleculeGPT #9710

wants to merge 39 commits into from

Conversation

xnuohz
Copy link
Contributor

@xnuohz xnuohz commented Oct 15, 2024

Issue

Feature Summary

@xnuohz xnuohz changed the title [WIP] MoleculeGPT Add MoleculeGPT Oct 21, 2024
@rusty1s
Copy link
Member

rusty1s commented Oct 22, 2024

@xnuohz Looks great. Can you do us a favor and split the PR into multiple? I would imagine we can merge dataset, model and example separately to ease reviewing.

@xnuohz
Copy link
Contributor Author

xnuohz commented Oct 23, 2024

@xnuohz Looks great. Can you do us a favor and split the PR into multiple? I would imagine we can merge dataset, model and example separately to ease reviewing.

@rusty1s Got it. I'll do this later.

@puririshi98
Copy link
Contributor

@xnuohz, notice CI fails because:
"E ModuleNotFoundError: No module named 'transformers'"
you need to add "@withPackage('transformers', 'sentencepiece', 'accelerate')"
Since the PyG github CI does not have these installed by default please manually test the unit test and share the results as a comment so we know everything works fine. At NVIDIA we do have CI that tests with these packages installed so once your work is merged it will be mantained :)

@xnuohz
Copy link
Contributor Author

xnuohz commented Oct 30, 2024

@puririshi98 Fixed CI and test the unit test locally.
image

@puririshi98
Copy link
Contributor

LGTM

@puririshi98
Copy link
Contributor

will wait for @rusty1s and @akihironitta to review/merge

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.

4 participants