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

Implementing Neural Fingerprint from Duvenaud et al. #7919

Merged
merged 26 commits into from
Sep 12, 2023

Conversation

harshit5674
Copy link
Contributor

@harshit5674 harshit5674 commented Aug 23, 2023

Implemented the Neural Fingerprint Model as suggested in the paper "Convolutional Networks on Graphs for Learning Molecular Fingerprints" and suggested in the issue #6077.

Unit test has been added and all tests are passing.

@codecov
Copy link

codecov bot commented Aug 23, 2023

Codecov Report

Merging #7919 (7a2740c) into master (297f9e6) will decrease coverage by 0.69%.
The diff coverage is 100.00%.

❗ Current head 7a2740c differs from pull request most recent head 0608f3d. Consider uploading reports for the commit 0608f3d to get more accurate results

@@            Coverage Diff             @@
##           master    #7919      +/-   ##
==========================================
- Coverage   90.31%   89.62%   -0.69%     
==========================================
  Files         462      463       +1     
  Lines       27023    27043      +20     
==========================================
- Hits        24405    24237     -168     
- Misses       2618     2806     +188     
Files Changed Coverage Δ
torch_geometric/nn/models/__init__.py 100.00% <100.00%> (ø)
torch_geometric/nn/models/neural_fingerprint.py 100.00% <100.00%> (ø)

... and 32 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@harshit5674
Copy link
Contributor Author

@wsad1 I request you to please review the PR.

@harshit5674
Copy link
Contributor Author

@wsad1 @EdisonLeeeee I request you to please review the PR.

Copy link
Member

@akihironitta akihironitta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@harshit5674 Thank you for working on this! I will have another look tomorrow :)

torch_geometric/nn/models/neural_fingerprint.py Outdated Show resolved Hide resolved
Copy link
Contributor

@EdisonLeeeee EdisonLeeeee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your work. Left some comments :)

torch_geometric/nn/models/neural_fingerprint.py Outdated Show resolved Hide resolved
torch_geometric/nn/models/neural_fingerprint.py Outdated Show resolved Hide resolved
torch_geometric/nn/models/neural_fingerprint.py Outdated Show resolved Hide resolved
torch_geometric/nn/models/neural_fingerprint.py Outdated Show resolved Hide resolved
torch_geometric/nn/models/neural_fingerprint.py Outdated Show resolved Hide resolved
test/nn/models/test_neural_fingerprint.py Outdated Show resolved Hide resolved
torch_geometric/nn/models/neural_fingerprint.py Outdated Show resolved Hide resolved
torch_geometric/nn/models/neural_fingerprint.py Outdated Show resolved Hide resolved
torch_geometric/nn/models/neural_fingerprint.py Outdated Show resolved Hide resolved
@rusty1s rusty1s linked an issue Sep 12, 2023 that may be closed by this pull request
@rusty1s rusty1s enabled auto-merge (squash) September 12, 2023 07:27
@rusty1s rusty1s disabled auto-merge September 12, 2023 07:31
@rusty1s rusty1s enabled auto-merge (squash) September 12, 2023 07:34
@rusty1s rusty1s merged commit b904044 into pyg-team:master Sep 12, 2023
JakubPietrakIntel pushed a commit that referenced this pull request Sep 27, 2023
Implemented the Neural Fingerprint Model as suggested in the paper
"[Convolutional Networks on Graphs for Learning Molecular
Fingerprints](https://arxiv.org/pdf/1509.09292.pdf)" and suggested in
the issue #6077.

Unit test has been added and all tests are passing.

---------

Co-authored-by: Harshit Verma <harshit@Harshits-MacBook-Air.local>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Jintang Li <cnljt@outlook.com>
Co-authored-by: rusty1s <matthias.fey@tu-dortmund.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement Neural Fingerprint from Duvenaud et al.
4 participants