Implement DeepDrug model #68

kajocina · 2022-02-01T18:25:55Z

Closes #14

Added the DeepDrug model, trying to copy as much as possible from their approach. The paper didn't use any context features, but I implemented the model so that it could be used in both ways. I ran the model with and without context feats on DrugCombDB with around 0.76 AUROC (no context features led to a minor drop in AUROC).

Provided an example that runs the model and a unit test which tests both the context and no-context modes of this model.

Unit tests provided for these changes
Documentation and docstrings added for these changes

cthoyt · 2022-02-01T18:27:39Z

@kajocina please pass CI before we review this PR. You can either read the results in the GitHub browser or run tox locally to see all issues that need to be fixed

chemicalx/models/deepdrug.py

cthoyt · 2022-02-01T21:22:52Z

I would suggest implementing this class only to have the functionality as originally described, then to consider subclassing it to have your additional functionality (adding in the context features may actually make it the same as one of the other models)

chemicalx/models/deepdrug.py

kajocina · 2022-02-02T11:13:36Z

@kajocina please pass CI before we review this PR. You can either read the results in the GitHub browser or run tox locally to see all issues that need to be fixed

@cthoyt I am getting CI errors from code in our tox dependencies so I can't really fix those, e.g.:

lint/bin/activate_this.py:28: error: "str" has no attribute "decode"; maybe "encode"?
lint/bin/activate_this.py:31: error: Module has no attribute "real_prefix"
Found 2 errors in 1 file (checked 39 source files)

I did fix tox issues related to my commits now.

cthoyt · 2022-02-02T11:33:34Z

@kajocina you can try running tox where it automatically rebuilds the virtualenvs with tox -r. If that's not going well you can even consider reinstalling tox itself. However, the CI run on GitHub actions still shows issues in the lint and documentation tasks. Please look into those

kajocina · 2022-02-02T15:07:42Z

@cthoyt thanks for the tip, rebuilt tox and works ok, local CI was green now. I added the wrapper as you suggested, but it didnt really decrease the number of lines of code in total, only the number of assignments performed.

cthoyt · 2022-02-02T15:08:56Z

@kajocina the goal was to make the model more legibile. I will demonstrate by pushing to your branch - see
1c60cae .

codecov-commenter · 2022-02-02T15:17:23Z

Codecov Report

Merging #68 (e459ba6) into main (5449f96) will increase coverage by 0.14%.
The diff coverage is 97.56%.

@@            Coverage Diff             @@
##             main      #68      +/-   ##
==========================================
+ Coverage   93.87%   94.01%   +0.14%     
==========================================
  Files          29       29              
  Lines         832      869      +37     
==========================================
+ Hits          781      817      +36     
- Misses         51       52       +1

Impacted Files	Coverage Δ
chemicalx/models/deepdrug.py	`96.77% <96.66%> (-3.23%)`	⬇️
tests/unit/test_models.py	`100.00% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5449f96...e459ba6. Read the comment docs.

benedekrozemberczki

The _helper functions are great.

cthoyt · 2022-02-02T15:49:49Z

Now that we have all of the implementations, I can see some interesting abstractions we can provide!

* WIP: model forward pass works, not tested * added dropout and batch norm * WIP: made DeepDrug example, not tested * moved to using layers only, not GCN torchdrug model * docstring * added dropout and made context feats optional * added DeepDrug unit test * deepdrug self attribute fix * docstring update * unpack method update (when no context feats used) * isort * fixed test setting (context_channels) * fixed testing without context * black * RST fix * RST fix * more pythonic loop + swap i to _ * removed context feat support in DeepDrug * removed context handling from testing DeepDrug * fixed examples DeepDrug, no context handling, decreased epochs 100->20 * removed unused import * used a wrapper for calling the same layers on pairs of batches * used a wrapper for calling the same layers on pairs of batches * docstring fix * Abstract process applied to left and right sides * Apply black * Cleanup Co-authored-by: Charles Tapley Hoyt <cthoyt@gmail.com>

* CASTER layer implementation - only supervised training stage - input dimensionality assumed to be correct * Apply black and reorganize * Move loss into its own module * Update caster.py * Reduce diff on citation * Implement DeepDrug model (#68) * WIP: model forward pass works, not tested * added dropout and batch norm * WIP: made DeepDrug example, not tested * moved to using layers only, not GCN torchdrug model * docstring * added dropout and made context feats optional * added DeepDrug unit test * deepdrug self attribute fix * docstring update * unpack method update (when no context feats used) * isort * fixed test setting (context_channels) * fixed testing without context * black * RST fix * RST fix * more pythonic loop + swap i to _ * removed context feat support in DeepDrug * removed context handling from testing DeepDrug * fixed examples DeepDrug, no context handling, decreased epochs 100->20 * removed unused import * used a wrapper for calling the same layers on pairs of batches * used a wrapper for calling the same layers on pairs of batches * docstring fix * Abstract process applied to left and right sides * Apply black * Cleanup Co-authored-by: Charles Tapley Hoyt <cthoyt@gmail.com> * Add GCN-BMP (#71) * linting * GCNBMP Scatter Reduction fix * Using Rel Conv Layers instead of RGCN Model (avoid unecessary sum readouts) * Added docstrings and fixed highway update implementation * Make number of relationship configurable * little help of black for linting * Cleaning upuseless imports * Sharing attention between right and left side * Adding reference to GCNBMP docstring * Type hinting everything * Fixing docstring in example * - Removing type hints in docstrings as they were added to signatures - Chunked iteration of the BMP backbone for better readability * Ading more-itertools as a dependecy * Using pairwise for encoder construction * Adding missing docstrings * Fixing linting and precommit hook * Fixing the citation back to what is in main * Tests,formatting,example * Tests,formatting,example * GCNBMP * Cleanup Co-authored-by: kcvc236 <kcvc236@seskscpg057.prim.scp> Co-authored-by: Rozemberczki <kmdb028@astrazeneca.net> Co-authored-by: kcvc236 <kcvc236@seskscpg059.prim.scp> Co-authored-by: Charles Tapley Hoyt <cthoyt@gmail.com> * Implement DeepDDI model (#63) * update: Add deepddi model * update: Add deepddi examples * update: Add deepddi test case * Style: deepddi model * Style: deepddi model * Style: deepddi_examples.py * Update deepddi.py * Update deepddi.py Co-authored-by: Charles Tapley Hoyt <cthoyt@gmail.com> * CASTER review fixes * flake8 fixes * CASTER: typing fix Co-authored-by: Andriy Nikolov <kgsq682@astrazeneca.net> Co-authored-by: Charles Tapley Hoyt <cthoyt@gmail.com> Co-authored-by: Piotr Grabowski <3966940+kajocina@users.noreply.github.com> Co-authored-by: Michaël Ughetto <michael.ughetto@astrazeneca.com> Co-authored-by: kcvc236 <kcvc236@seskscpg057.prim.scp> Co-authored-by: Rozemberczki <kmdb028@astrazeneca.net> Co-authored-by: kcvc236 <kcvc236@seskscpg059.prim.scp> Co-authored-by: walter <32014404+hzcheney@users.noreply.github.com>

kajocina added 13 commits February 1, 2022 18:20

WIP: model forward pass works, not tested

52f49d5

added dropout and batch norm

dcf73f5

WIP: made DeepDrug example, not tested

920fb16

moved to using layers only, not GCN torchdrug model

e2841c5

docstring

f094bf9

added dropout and made context feats optional

608d62e

added DeepDrug unit test

d4088c9

deepdrug self attribute fix

a75ed8f

docstring update

9ea62e3

unpack method update (when no context feats used)

e13232d

isort

1612465

fixed test setting (context_channels)

4e6ee48

fixed testing without context

30bd8b8

kajocina requested review from cthoyt and benedekrozemberczki February 1, 2022 18:25

cthoyt reviewed Feb 1, 2022

View reviewed changes

chemicalx/models/deepdrug.py Outdated Show resolved Hide resolved

cthoyt changed the title ~~14 add deepdrug model~~ Add DeepDrug model Feb 1, 2022

cthoyt added the model label Feb 1, 2022

cthoyt changed the title ~~Add DeepDrug model~~ Implement DeepDrug model Feb 1, 2022

cthoyt reviewed Feb 1, 2022

View reviewed changes

chemicalx/models/deepdrug.py Outdated Show resolved Hide resolved

cthoyt reviewed Feb 1, 2022

View reviewed changes

chemicalx/models/deepdrug.py Outdated Show resolved Hide resolved

cthoyt reviewed Feb 1, 2022

View reviewed changes

chemicalx/models/deepdrug.py Outdated Show resolved Hide resolved

kajocina added 6 commits February 2, 2022 09:50

black

7a95c88

RST fix

9ba5779

RST fix

91940af

more pythonic loop + swap i to _

6dc2e54

removed context feat support in DeepDrug

235107b

removed context handling from testing DeepDrug

a25335a

kajocina added 2 commits February 2, 2022 10:05

fixed examples DeepDrug, no context handling, decreased epochs 100->20

e1cc3d5

removed unused import

61aaf06

kajocina added 3 commits February 2, 2022 14:26

used a wrapper for calling the same layers on pairs of batches

2515139

used a wrapper for calling the same layers on pairs of batches

692a729

docstring fix

97f052d

cthoyt added 3 commits February 2, 2022 16:09

Merge branch 'main' into 14-add-deepdrug-model

205c793

Abstract process applied to left and right sides

1c60cae

Apply black

c93c903

cthoyt self-assigned this Feb 2, 2022

Cleanup

e459ba6

kajocina removed the request for review from benedekrozemberczki February 2, 2022 15:38

cthoyt approved these changes Feb 2, 2022

View reviewed changes

cthoyt requested a review from benedekrozemberczki February 2, 2022 15:42

benedekrozemberczki approved these changes Feb 2, 2022

View reviewed changes

benedekrozemberczki merged commit 2fb3421 into main Feb 2, 2022

benedekrozemberczki deleted the 14-add-deepdrug-model branch February 2, 2022 15:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement DeepDrug model #68

Implement DeepDrug model #68

kajocina commented Feb 1, 2022 •

edited by cthoyt

cthoyt commented Feb 1, 2022

cthoyt commented Feb 1, 2022

kajocina commented Feb 2, 2022

cthoyt commented Feb 2, 2022

kajocina commented Feb 2, 2022

cthoyt commented Feb 2, 2022 •

edited

codecov-commenter commented Feb 2, 2022 •

edited

benedekrozemberczki left a comment

cthoyt commented Feb 2, 2022

Implement DeepDrug model #68

Implement DeepDrug model #68

Conversation

kajocina commented Feb 1, 2022 • edited by cthoyt

cthoyt commented Feb 1, 2022

cthoyt commented Feb 1, 2022

kajocina commented Feb 2, 2022

cthoyt commented Feb 2, 2022

kajocina commented Feb 2, 2022

cthoyt commented Feb 2, 2022 • edited

codecov-commenter commented Feb 2, 2022 • edited

Codecov Report

benedekrozemberczki left a comment

Choose a reason for hiding this comment

cthoyt commented Feb 2, 2022

kajocina commented Feb 1, 2022 •

edited by cthoyt

cthoyt commented Feb 2, 2022 •

edited

codecov-commenter commented Feb 2, 2022 •

edited