Implement DeepDDI model #63

hzcheney · 2022-01-28T16:54:12Z

Closes #2

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

Changes

add DeepDDI model
add DeepDDI examples
add DeepDDI testcase

chemicalx/models/deepddi.py

cthoyt · 2022-01-28T17:07:17Z

Please run tox locally to make sure that all tests are passing before we give a further review

codecov-commenter · 2022-01-28T17:08:08Z

Codecov Report

Merging #63 (c16f927) into main (5449f96) will increase coverage by 0.06%.
The diff coverage is 96.66%.

@@            Coverage Diff             @@
##             main      #63      +/-   ##
==========================================
+ Coverage   93.87%   93.93%   +0.06%     
==========================================
  Files          29       29              
  Lines         832      858      +26     
==========================================
+ Hits          781      806      +25     
- Misses         51       52       +1

Impacted Files	Coverage Δ
chemicalx/models/deepddi.py	`95.00% <94.73%> (-5.00%)`	⬇️
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...c16f927. Read the comment docs.

benedekrozemberczki · 2022-01-30T10:50:31Z

Hi there @hzcheney, do you plan to update this PR?

hzcheney · 2022-01-30T13:49:54Z

Hi there @hzcheney, do you plan to update this PR?

Yes! I will update this PR ASAP.

benedekrozemberczki · 2022-01-30T17:10:24Z

The formatting still fails.

benedekrozemberczki · 2022-01-30T17:15:23Z

These are the errors:

./tests/unit/test_models.py:61:59: BLK100 Black would make changes.
./examples/deepddi_examples.py:22:10: BLK100 Black would make changes.
./examples/epgcnds_examples.py:15:55: BLK100 Black would make changes.

hzcheney · 2022-01-30T17:18:32Z

I am working on it, it's weird that I run tox locally well.

hzcheney · 2022-01-30T17:50:21Z

Please help! I have no idea why these errors still happened:

./tests/unit/test_models.py:61:59: BLK100 Black would make changes.
./examples/epgcnds_examples.py:15:55: BLK100 Black would make changes.

Even I did not change the epgcnds_examples.py file.
:-)

cthoyt · 2022-01-30T17:53:45Z

Yesterday, black made a new release that will cause some changes. The options are either:

Wait for a new PR where I make these changes
Pin black (terrible idea! Do not do this!)
Commit the changes black makes when you run tox

hzcheney · 2022-01-30T17:58:11Z

Yesterday, black made a new release that will cause some changes. The options are either:

Wait for a new PR where I make these changes

Pin black (terrible idea! Do not do this!)

Commit the changes black makes when you run tox

Thank you! It's clear to me now.

chemicalx/models/deepddi.py

examples/deepddi_examples.py

benedekrozemberczki · 2022-02-01T12:20:53Z

Hi there @hzcheney, I suggest updating black on your side. Do you want to finish the PR or we can overtake from you?

benedekrozemberczki

@cthoyt can you make these changes?

benedekrozemberczki · 2022-01-28T17:00:45Z

chemicalx/models/deepddi.py

 __all__ = [
    "DeepDDI",
 ]


-class DeepDDI(UnimplementedModel):
+class DeepDDI(Model):
    """An implementation of the DeepDDI model.


Please change the docstring to include paper reference.

benedekrozemberczki · 2022-01-28T17:01:15Z

chemicalx/models/deepddi.py

+        *,
+        drug_channels: int,
+        hidden_channels: int = 2048,
+        hidden_layers_num: int = 9,


Can we setup a smaller number of layers and lower hidden channel number?

@cthoyt can we have 4 layer with 32 hidden channels?

benedekrozemberczki · 2022-01-28T17:04:14Z

chemicalx/models/deepddi.py

+
+        super(DeepDDI, self).__init__()
+        assert hidden_layers_num > 1
+        dnn = []


I would strongly suggest using a fixed number of layers. E.g. 3 or 4.

benedekrozemberczki · 2022-01-28T17:05:34Z

chemicalx/models/deepddi.py

+        input_feature = torch.cat([drug_features_left, drug_features_right], 1)
+        hidden = self.dnn(input_feature)
+        return hidden
+


Remove spaces. The PR does not follow linting. Please use black to format your code. When you push again you can test it on the repo.

benedekrozemberczki · 2022-01-28T17:06:20Z

tests/unit/test_models.py

@@ -165,8 +165,22 @@ def test_ssiddi(self):

    def test_deepddi(self):
        """Test DeepDDI."""
-        model = DeepDDI(x=2)
-        assert model.x == 2


Create a test also for hidden_layer_num=1.

* 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 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>

hzcheney added 3 commits January 29, 2022 00:45

update: Add deepddi model

8e7466d

update: Add deepddi examples

e485d06

update: Add deepddi test case

a1c661e

cthoyt reviewed Jan 28, 2022

View reviewed changes

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

cthoyt reviewed Jan 28, 2022

View reviewed changes

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

Style: deepddi model

7e86de4

Style: deepddi model

20f0a50

Style: deepddi_examples.py

da0bb06

cthoyt reviewed Jan 30, 2022

View reviewed changes

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

cthoyt reviewed Jan 30, 2022

View reviewed changes

examples/deepddi_examples.py Show resolved Hide resolved

cthoyt added the model label Feb 1, 2022

cthoyt changed the title ~~Implementation of DeepDDI model~~ Implement DeepDDI model Feb 1, 2022

cthoyt added 2 commits February 2, 2022 16:04

Merge branch 'main' into pr/63

3c74d7c

Update deepddi.py

7b631a5

cthoyt approved these changes Feb 2, 2022

View reviewed changes

Update deepddi.py

c16f927

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

cthoyt self-assigned this Feb 2, 2022

benedekrozemberczki suggested changes Feb 2, 2022

View reviewed changes

benedekrozemberczki approved these changes Feb 2, 2022

View reviewed changes

benedekrozemberczki merged commit 424b440 into AstraZeneca:main Feb 2, 2022

hzcheney deleted the issue2 branch February 4, 2022 09:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement DeepDDI model #63

Implement DeepDDI model #63

hzcheney commented Jan 28, 2022 •

edited by cthoyt

Loading

cthoyt commented Jan 28, 2022

codecov-commenter commented Jan 28, 2022 •

edited

Loading

benedekrozemberczki commented Jan 30, 2022

hzcheney commented Jan 30, 2022

benedekrozemberczki commented Jan 30, 2022

benedekrozemberczki commented Jan 30, 2022

hzcheney commented Jan 30, 2022

hzcheney commented Jan 30, 2022 •

edited

Loading

cthoyt commented Jan 30, 2022

hzcheney commented Jan 30, 2022

benedekrozemberczki commented Feb 1, 2022

benedekrozemberczki left a comment

benedekrozemberczki Jan 28, 2022

benedekrozemberczki Jan 28, 2022

benedekrozemberczki Feb 2, 2022

benedekrozemberczki Jan 28, 2022

benedekrozemberczki Jan 28, 2022

benedekrozemberczki Jan 28, 2022

Implement DeepDDI model #63

Implement DeepDDI model #63

Conversation

hzcheney commented Jan 28, 2022 • edited by cthoyt Loading

Changes

cthoyt commented Jan 28, 2022

codecov-commenter commented Jan 28, 2022 • edited Loading

Codecov Report

benedekrozemberczki commented Jan 30, 2022

hzcheney commented Jan 30, 2022

benedekrozemberczki commented Jan 30, 2022

benedekrozemberczki commented Jan 30, 2022

hzcheney commented Jan 30, 2022

hzcheney commented Jan 30, 2022 • edited Loading

cthoyt commented Jan 30, 2022

hzcheney commented Jan 30, 2022

benedekrozemberczki commented Feb 1, 2022

benedekrozemberczki left a comment

Choose a reason for hiding this comment

benedekrozemberczki Jan 28, 2022

Choose a reason for hiding this comment

benedekrozemberczki Jan 28, 2022

Choose a reason for hiding this comment

benedekrozemberczki Feb 2, 2022

Choose a reason for hiding this comment

benedekrozemberczki Jan 28, 2022

Choose a reason for hiding this comment

benedekrozemberczki Jan 28, 2022

Choose a reason for hiding this comment

benedekrozemberczki Jan 28, 2022

Choose a reason for hiding this comment

hzcheney commented Jan 28, 2022 •

edited by cthoyt

Loading

codecov-commenter commented Jan 28, 2022 •

edited

Loading

hzcheney commented Jan 30, 2022 •

edited

Loading