Healthcare and life science use case: retrosynthetic planning using reinforcement learning #121

AoyuQC · 2023-11-26T09:07:33Z

Description of changes:

Refactor the code in aws solution (https://aws.amazon.com/solutions/implementations/quantum-computing-exploration-for-drug-discovery/) and contribute to braket algorithm library. This is the first use case, the use of quantum circuit in reinforcement learning to solve the problem of retrosynthetic planning

Testing done:

Merge Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your pull request.

General

I have read the CONTRIBUTING doc
I used the commit message format described in CONTRIBUTING
I have updated any necessary documentation, including READMEs and API docs (if appropriate)

Tests

I have added tests that prove my fix is effective or that my feature works (if appropriate)
I have checked that my tests are not configured for a specific region or account (if appropriate)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

rmshaffer · 2023-11-28T17:58:53Z

@AoyuQC, quick clarification - is this PR ready for review? If yes, would you please remove "WIP" from the title to indicate this? If no, would you please convert the PR to draft until it is ready? Thank you!

AoyuQC · 2023-12-11T03:00:47Z

@AoyuQC, quick clarification - is this PR ready for review? If yes, would you please remove "WIP" from the title to indicate this? If no, would you please convert the PR to draft until it is ready? Thank you!

Hi, I will convert it to draft

data prepare code

sussii · 2024-01-19T02:46:27Z

Confirmed with Aoyu (the author). The PR is ready for review. Could someone please take a look? Thanks!

AoyuQC · 2024-01-19T04:46:40Z

@rmshaffer , please change it from draft to ready to review status. Thank you!

rmshaffer

I haven't reviewed the content of the algorithm or notebook, but I left a few initial comments on the code structure and formatting. To enable the CI to pass, please ensure that you can run the successfully following commands locally:

black . and flake8 ➡️ checks code formatting; please fix any failures reported
tox -e unit-tests ➡️ runs unit tests; please ensure that your new test runs successfully in this method

rmshaffer · 2024-01-22T16:34:31Z

...s/industry_applications/healthcare-and-life-sciences/Retrosynthetic_Planning/requirement.txt

Should this file be named requirements.txt?

rmshaffer · 2024-01-22T16:35:35Z

src/braket/experimental/algorithms/qc_qrl/__init__.py

+# Copyright Amazon.com Inc. or its affiliates. All Rights Reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License"). You
+# may not use this file except in compliance with the License. A copy of
+# the License is located at
+#
+#     http://aws.amazon.com/apache2.0/
+#
+# or in the "license" file accompanying this file. This file is
+# distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF
+# ANY KIND, either express or implied. See the License for the specific
+# language governing permissions and limitations under the License.


If your goal is to use this copyright/license for the PR, can you please ensure this is at the top of every new .py file you've added?

rmshaffer · 2024-01-22T16:38:08Z

src/braket/experimental/algorithms/qc_qrl/retrorl_algorithm_script.py

+# os.system(f"aws s3 cp {save_path} s3://{s3}/data/{save_name}")
+# AWS_REGION = "us-west-1"
+# S3_BUCKET_NAME = "amazon-braket-us-west-1-493904798517"
+# s3_client = boto3.client("s3", region_name=AWS_REGION)
+# s3_client.upload_file(save_path, S3_BUCKET_NAME, f'data/{save_name}')
+# job_name = os.environ["AMZN_BRAKET_JOB_NAME"]
+# save_job_checkpoint(
+#     checkpoint_data={"data": f"data for checkpoint from {job_name}"},
+#     checkpoint_file_suffix="checkpoint-1",
+#     )
+# if retro_rl_agent.name.split('_')[1] == 'aspen-m2':
+#     AWS_REGION = "us-west-1"
+#     S3_BUCKET_NAME = "amazon-braket-us-west-1-493904798517"
+#     s3_client = boto3.client("s3", region_name=AWS_REGION)
+#     s3_client.upload_file(save_path, S3_BUCKET_NAME, f'data/{save_name}')
+# else:
+#     AWS_REGION = "us-west-1"
+#     S3_BUCKET_NAME = "amazon-braket-us-west-1-493904798517"
+#     s3_client = boto3.client("s3", region_name=AWS_REGION)
+#     s3_client.upload_file(save_path, S3_BUCKET_NAME, f'data/{save_name}')


This is a general comment that applies to the whole PR - it's best practice to avoid merging code that is commented-out. (For a few reasons: it's not testable; it will likely break in the future, even if it works now; and it's not clear to a reader why the code is there.) For each commented block of code, is there a good reason to keep the code? If so, is there an alternative to having it commented out (for example, put it behind a flag that the user can optionally enable)?

rmshaffer · 2024-01-22T16:45:14Z

...ciences/Retrosynthetic_Planning/Retrosynthetic_Planning_Quantum_Reinforcement_Learning.ipynb

Would you mind doing a quick spell-check on the notebook content?

Also, if you would like to run automatic Python code formatting on this notebook, you can use black for this:

pip install "black[jupyter]"

black ./notebooks/industry_applications/healthcare-and-life-sciences/Retrosynthetic_Planning/Retrosynthetic_Planning_Quantum_Reinforcement_Learning.ipynb

ykharkov · 2024-01-23T22:23:01Z

Hi @AoyuQC ,

thanks for the PR.

I have a few comments from the scientific perspective.
The industrial use case considered here is very interesting indeed and would be a great add-on to our Algorithm library.

However, I have a few concerns about the results obtained in the notebook and their interpretation:

The notebook does not explain the meaning of a "classical circuit" (plot at the end of notebook), which is supposed to be a classical benchmark. It is totally unclear what the classical baseline is right now. The existing "classical circuit" baseline only makes the Cost function worse throughout training, and definitely it is of a major concern.
The author references the paper https://pubs.acs.org/doi/10.1021/acscentsci.9b00055, and I strongly recommend to use their method as a baseline. From Fig. 3 of the referenced paper it is clear, that the classical baseline improves the Cost function during training. So the narrative of the notebook that quantum RL model is outperforming a classical method is not substantiated. Perhaps if a direct comparison with classical methods is problematic, I could recommend to remove this comparison and to not make any claims that quantum has an advantage over classical approach.
The code in RetroRLAgent.py and RetroGateModel.py is totally unreadable, while these the key files here. The files contain a lot of commented out code, and nobody opening these files would be able to understand (even on a high level) what is going on. I would strongly suggest to revise the code in these two files and improve the quality / readability, otherwise unfortunately it can not pass Braket quality bar.

flake8 solutions

peterkomar-aws · 2024-03-29T18:33:26Z

Hello AoyuQC,

The algorithm you are proposing to add has many external dependencies, and it explicitly relies on the user running the terminal instructions from the notebook. It's clear that the nature of this use case necessitates these dependencies, but as maintainers of the Braket Algorithm library, we wish to make the dependencies minimal and, more importantly, explicit.

After reviewing your PR, we think, in its current form, it is better suited to be developed in a separate repo; and instead, we'd love to feature it in the Braket Algorithm library "Community repos" section, which we recently added.

I will close this PR for now, but feel free to continue to work on it here and reopen.

AoyuQC · 2024-04-03T07:39:38Z

Hello AoyuQC,

The algorithm you are proposing to add has many external dependencies, and it explicitly relies on the user running the terminal instructions from the notebook. It's clear that the nature of this use case necessitates these dependencies, but as maintainers of the Braket Algorithm library, we wish to make the dependencies minimal and, more importantly, explicit.

After reviewing your PR, we think, in its current form, it is better suited to be developed in a separate repo; and instead, we'd love to feature it in the Braket Algorithm library "Community repos" section, which we recently added.

I will close this PR for now, but feel free to continue to work on it here and reopen.

Hi @peterkomar-aws ,

Thank you for the update. I am working on it.

Best,

Aoyu

AoyuQC added 3 commits November 26, 2023 06:26

initial for retro

d8b2f08

feat: add retro complete folders

3551bb9

fix: add test

abb9f13

AoyuQC changed the title ~~WIP healthcare and life science use case: retrosynthetic planning using reinforcement learning~~ WIP: healthcare and life science use case: retrosynthetic planning using reinforcement learning Nov 26, 2023

rmshaffer marked this pull request as draft December 11, 2023 17:12

AoyuQC and others added 9 commits December 21, 2023 13:33

Merge branch 'main' into feature/hcls-app-retrosynthetic

c08b84a

feat: add python dotenv

2f2b0e1

feat: add nb to displace difference between qc and cc

aba4fae

feat: ready to review

f1ae9f7

data prepare code

b0ea23c

update to lightning.gpu

100ba38

Merge pull request #1 from lxy-z/feature/hcls-app-retrosynthetic

2acb327

data prepare code

add logic to check whether there is cpu/gpu

fb41642

add txt ddescription

5f8904d

AoyuQC changed the title ~~WIP: healthcare and life science use case: retrosynthetic planning using reinforcement learning~~ Healthcare and life science use case: retrosynthetic planning using reinforcement learning Jan 15, 2024

rmshaffer marked this pull request as ready for review January 22, 2024 15:30

rmshaffer reviewed Jan 22, 2024

View reviewed changes

AoyuQC and others added 6 commits January 29, 2024 13:47

feat: finish black .

19457e9

fix flake8 check, only left too complex issue

a49f8b4

Merge branch 'main' into feature/hcls-app-retrosynthetic

8a30aad

1

8891f1b

Merge pull request #3 from lxy-z/feature/hcls-app-retrosynthetic

3270724

flake8 solutions

reformat done

d3bb85c

amazon-braket deleted a comment from Alireza-Rahimi-3000 Mar 28, 2024

peterkomar-aws closed this Mar 29, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Healthcare and life science use case: retrosynthetic planning using reinforcement learning #121

Healthcare and life science use case: retrosynthetic planning using reinforcement learning #121

AoyuQC commented Nov 26, 2023

rmshaffer commented Nov 28, 2023

AoyuQC commented Dec 11, 2023

sussii commented Jan 19, 2024

AoyuQC commented Jan 19, 2024

rmshaffer left a comment

rmshaffer Jan 22, 2024

rmshaffer Jan 22, 2024

rmshaffer Jan 22, 2024

rmshaffer Jan 22, 2024

rmshaffer Jan 22, 2024

ykharkov commented Jan 23, 2024 •

edited

Loading

peterkomar-aws commented Mar 29, 2024

AoyuQC commented Apr 3, 2024

Healthcare and life science use case: retrosynthetic planning using reinforcement learning #121

Healthcare and life science use case: retrosynthetic planning using reinforcement learning #121

Conversation

AoyuQC commented Nov 26, 2023

Merge Checklist

General

Tests

rmshaffer commented Nov 28, 2023

AoyuQC commented Dec 11, 2023

sussii commented Jan 19, 2024

AoyuQC commented Jan 19, 2024

rmshaffer left a comment

Choose a reason for hiding this comment

rmshaffer Jan 22, 2024

Choose a reason for hiding this comment

rmshaffer Jan 22, 2024

Choose a reason for hiding this comment

rmshaffer Jan 22, 2024

Choose a reason for hiding this comment

rmshaffer Jan 22, 2024

Choose a reason for hiding this comment

rmshaffer Jan 22, 2024

Choose a reason for hiding this comment

ykharkov commented Jan 23, 2024 • edited Loading

peterkomar-aws commented Mar 29, 2024

AoyuQC commented Apr 3, 2024

ykharkov commented Jan 23, 2024 •

edited

Loading