Install & Update Packages Dependencies #512

AI-Ahmed · 2022-11-11T20:25:10Z

Install a Specific version of Scipy scipy==1.1.0 Library to fix the error in Colab. Also, Update the downloaded GYM library on Colab and install gym for Atari & ROM Licence after downloading xvfb.

Install Specific version of Scipy Library to fix the error in Colab. Also, Intstalled GYM library for Atari & ROM Licence after downloading xvfb

review-notebook-app · 2022-11-11T20:25:14Z

Check out this pull request on

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

Merge with the base

Download and install the video game dependencies, `ROM`, and install `xvfb` correctly. Downgrade the `gym` environment for monitoring the game.

Add the video lecture of Pavel Shvechikov (yandexdataschool#510)

week06_policy_based/a2c-optional.ipynb

week08_pomdp/practice_pytorch.ipynb

Update the hash of the imported scripts of `atari_util.py` and `env_pool.py`.

AI-Ahmed

Updated the hash and the typo version of gym

Added new monitoring script and reward scaling for the reward for having better training performance with reducing the gradient exploration.

AI-Ahmed

Thanks for reviewing the notebook, but it doesn't appear where you have committed your changes since the file is not rendering, and the title doesn't tell. Would you mind if you tell me the simplified diff you made, please?

AI-Ahmed

Also, I see you deleted lots of things!

dniku

Please take a look at my suggestions.

dniku · 2022-12-04T19:55:40Z

week06_policy_based/a2c-optional.ipynb

-     "    os.system('wget https://raw.githubusercontent.com/yandexdataschool/Practical_RL/master/xvfb -O ../xvfb')\n",
+     "    # Download and install Video game dependencies\n",
+     "    os.system('wget -q https://raw.githubusercontent.com/yandexdataschool/Practical_RL/master/setup_colab.sh -O- | bash')\n",
+     "    os.system('touch .setup_complete')\n",


This file is meant to be an indicator that the entire setup block does not need to be re-run. Please put its creation at the bottom of this block.

It is a bit weird the file is placed because this is what I have in my repos!!

Maybe this is an old version...

dniku · 2022-12-04T19:56:01Z

week08_pomdp/practice_pytorch.ipynb

-    "\n",
+    "    !wget https://raw.githubusercontent.com/yandexdataschool/Practical_RL/f1d7764b276cadb7365b1fdb6f6dd3fbd4e7bd8d/week08_pomdp/atari_util.py\n",
+    "    !wget https://raw.githubusercontent.com/yandexdataschool/Practical_RL/f1d7764b276cadb7365b1fdb6f6dd3fbd4e7bd8d/week08_pomdp/env_pool.py\n",
+    "    # Setup the attari driver for video games\n",


Please remove this comment.

Which comment are you referring to?

dniku · 2022-12-04T19:56:15Z

week08_pomdp/practice_pytorch.ipynb

+    "    !wget https://raw.githubusercontent.com/yandexdataschool/Practical_RL/f1d7764b276cadb7365b1fdb6f6dd3fbd4e7bd8d/week08_pomdp/env_pool.py\n",
+    "    # Setup the attari driver for video games\n",
+    "    !wget -q https://raw.githubusercontent.com/yandexdataschool/Practical_RL/master/setup_colab.sh -O- | bash\n",
+    "    !touch .setup_complete\n",


Please put creation of this file at the bottom of this block.

I checked the shell file and didn't see any problem with having it in its current place!

dniku · 2022-12-04T19:57:25Z

week08_pomdp/practice_pytorch.ipynb

+    "# Restart the Runtime\n",
+    "print(\"Restarting the Runtime...\")\n",
+    "os.kill(os.getpid(), 9)"


I very much doubt that this is necessary. Something like this should only be required if you modify the libraries that you import — but here, instead, you just install more libraries. It also prevents the user from using the "Restart & Run All" feature in Jupyter and Colab.

Please remove this.

You are correct! We are modifying the package of Gym by adding more extensions, such as [atari,accept-rom-license] so that we have to restart the kernel. I have tested it without the kernel killing process, and it showed me an error in gym environment.

dniku · 2022-12-04T19:58:06Z

week08_pomdp/practice_pytorch.ipynb

@@ -25,9 +42,11 @@
   "metadata": {},
   "outputs": [],
   "source": [
+    "import os\n",


os has been imported previously.

Yes! But after the restart of the kernel, I need to call it again!

dniku · 2022-12-04T20:20:02Z

week08_pomdp/practice_pytorch.ipynb

-    "            break"
+    "rollout_len = 10\n",
+    "# Change to higher number of steps after you ensure you get progress\n",
+    "STEPS = int(5e+4) #int(15e+3)\n",


5e+4 and 15e+3 is a lot harder to read than 50_000 and 15_000. Please fix.

dniku · 2022-12-04T20:23:40Z

week08_pomdp/practice_pytorch.ipynb

+    "  if i % 500 == 0:\n",
+    "    rollout_len = min(40, rollout_len + 1)\n",
+    "    print(f\"\\nNumber of Interactions per steps: {rollout_len}\")"


It's worth pointing out explicitly (in a Markdown text block) that there is this rollout length schedule and the rationale for it.

I think you guys have already done that!? And that's why I added it!!

dniku · 2022-12-04T20:23:58Z

week08_pomdp/practice_pytorch.ipynb

+    "        print(\"\\nYour agent has just passed the minimum homework threshold with score: \", rewards_history[-1])\n",
+    "        break\n",
+    "      elif 8000 <= rewards_history[-1] <= 8499:\n",
+    "        print(\"\\nYour agent has just get 'Red' Belt with score: \", rewards_history[-1])\n",


has just got

dniku · 2022-12-04T20:24:04Z

week08_pomdp/practice_pytorch.ipynb

+    "      elif 8000 <= rewards_history[-1] <= 8499:\n",
+    "        print(\"\\nYour agent has just get 'Red' Belt with score: \", rewards_history[-1])\n",
+    "      elif 8500 <= rewards_history[-1] <= 9999:\n",
+    "        print(\"\\nYour agent has just get 'Red-Black' Belt with score: \", rewards_history[-1])\n",


has just got

dniku · 2022-12-04T20:26:09Z

week08_pomdp/practice_pytorch.ipynb

+   "metadata": {},
+   "outputs": [],
+   "source": [
+    "torch.save(agent, \"LOCATION TO SAVE YOUR AGENT\")"


Please extract the checkpoint path into a separate variable such as checkpoint_path.

This is not a good way to save PyTorch model weights. Please use torch.save(agent.state_dict()) instead.

dniku · 2022-12-04T20:31:03Z

My commit is basically a reformatting of the week08_pomdp/practice_pytorch.ipynb notebook that enables me to see the actual diff in the "Files changed" tab. Here is the exact script that I used to do that:

#!/usr/bin/env python3

import argparse
import subprocess
import sys
from pathlib import Path

import nbformat


def upgrade_notebook_version(path, new_version=4):
    # Documentation: https://nbformat.readthedocs.io/en/latest/api.html
    with path.open('r') as fp:
        nb = nbformat.read(fp, new_version)

    with path.open('w') as fp:
        nbformat.write(nb, fp)


def cleanup_fields(path, clear_outputs, notebook_full_metadata=False):
    jq_cmd = [
        # Remove execution count from inputs
        '(.cells[] | select(has("execution_count")) | .execution_count) = null',
        # Remove execution count from outputs
        '(.cells[] | select(has("outputs")) | .outputs[] | select(has("execution_count")) | .execution_count) = null',
        # Remove cell metadata
        '.cells[].metadata = {}',
    ]

    # Standardize notebook metadata
    if notebook_full_metadata:
        jq_cmd.append(
            '.metadata = {' +
                '"kernelspec": {' +
                    '"display_name": "Python 3", ' +
                    '"language": "python", ' +
                    '"name": "python3"' +
                '}, ' +
                '"language_info": {' +
                    '"codemirror_mode": {"name": "ipython", "version": 3}, ' +
                    '"file_extension": ".py", ' +
                    '"mimetype": "text/x-python", ' +
                    '"name": "python", ' +
                    '"nbconvert_exporter": "python", ' +
                    '"pygments_lexer": "ipython3"' +
                '}' +
            '}'
        )
    else:
        jq_cmd.append(
            '.metadata = {"language_info": {"name": "python", "pygments_lexer": "ipython3"}}'
        )


    if clear_outputs:
        jq_cmd.append(
            '(.cells[] | select(has("outputs")) | .outputs) = []'
        )

    cmd = [
        'jq',
        '--indent', '1',
        ' | '.join(jq_cmd),
        str(path),
    ]

    formatted = subprocess.check_output(cmd, encoding='utf8')

    with path.open('w') as fp:
        fp.write(formatted)


def main():
    parser = argparse.ArgumentParser()
    parser.add_argument('path', type=Path)
    parser.add_argument('--clear-outputs', action='store_true', help='Clear outputs of all cells')
    parser.add_argument('--no-cleanup', action='store_true', help='Do not cleanup JSON at all')
    args = parser.parse_args()

    upgrade_notebook_version(args.path)
    if not args.no_cleanup:
        cleanup_fields(args.path, args.clear_outputs)
    else:
        assert not args.clear_outputs


if __name__ == '__main__':
    main()

and the command was pretty-ipynb --clear-outputs week08_pomdp/practice_pytorch.ipynb.

AI-Ahmed

Man!!! That was a long discussion for a file! You can ask me to give you access to the file on Colab to test it out instead!! 😂

AI-Ahmed · 2022-12-05T18:40:42Z

week08_pomdp/practice_pytorch.ipynb

@@ -193,11 +193,11 @@
    "        # Apply the whole neural net for one step here.\n",
    "        # See docs on self.rnn(...).\n",
    "        # The recurrent cell should take the last feedforward dense layer as input.\n",
-    "        <YOUR CODE>\n",
+    "        # <YOUR CODE>\n",


I will adjust this also

AI-Ahmed · 2022-12-05T18:43:28Z

week08_pomdp/practice_pytorch.ipynb

@@ -246,8 +246,7 @@
   "cell_type": "markdown",
   "metadata": {},
   "source": [
-    "### Let's play!\n",
-    "Let's build a function that measures agent's average reward."


Weirdly, I have them such as

AI-Ahmed · 2022-12-05T18:45:18Z

week08_pomdp/practice_pytorch.ipynb

+    "    if record:\n",
+    "      if record ==\"Statistic\":\n",


I have already added a typing identification for that!

AI-Ahmed · 2022-12-05T18:46:59Z

week08_pomdp/practice_pytorch.ipynb

@@ -278,7 +294,7 @@
    "            if done:\n",
    "                break\n",
    "\n",
-    "        game_rewards.append(total_reward)\n",
+    "        game_rewards.append(total_reward / reward_scale)\n",


The whole logic will be illogical if it is parameterised since we need reward scaling.

AI-Ahmed · 2022-12-05T18:49:50Z

week08_pomdp/practice_pytorch.ipynb

@@ -304,7 +322,7 @@
   "source": [
    "# Show video. This may not work in some setups. If it doesn't\n",
    "# work for you, you can download the videos and view them locally.\n",
-    "\n",
+    "import sys\n",


Weirdly, I don't have that!!!

Do you think I have to submit the 13 commits ahead of the main repository again? Because of these problems?

AI-Ahmed · 2022-12-05T19:15:10Z

week08_pomdp/practice_pytorch.ipynb

+    "# adding lambda λ to make compromise between bias and variance.\n",
+    "lamda = 0.92 # best value according to (Schulman et al., 2018) if you are going to work with GAE\n",
+    "ENT_COEF = 0.01\n",
+    "GRADIENT_COEF = 0.5"


AI-Ahmed · 2022-12-05T19:15:53Z

week08_pomdp/practice_pytorch.ipynb

+    "set_seed()\n",
+    "rewards_history, grad_norm_history, entropy_history, entropy_loss_history, loss_history, Jhat, v_loss = [],[],[],[],[],[],[]\n",
+    "agent = SimpleRecurrentAgent(obs_shape, n_actions)\n",
+    "opt = torch.optim.Adam(agent.parameters(), lr=1e-5)\n"


What??? It is?!!!

AI-Ahmed · 2022-12-05T19:25:19Z

week08_pomdp/practice_pytorch.ipynb

+    "  if i % 500 == 0:\n",
+    "    rollout_len = min(40, rollout_len + 1)\n",
+    "    print(f\"\\nNumber of Interactions per steps: {rollout_len}\")"


I think you guys have already done that!? And that's why I added it!!

AI-Ahmed · 2022-12-05T19:26:17Z

week08_pomdp/practice_pytorch.ipynb

+   "metadata": {},
+   "outputs": [],
+   "source": [
+    "torch.save(agent, \"LOCATION TO SAVE YOUR AGENT\")"


AI-Ahmed · 2022-12-05T19:29:53Z

week08_pomdp/practice_pytorch.ipynb

+   "metadata": {},
+   "outputs": [],
+   "source": [
+    "torch.save(agent, \"LOCATION TO SAVE YOUR AGENT\")"


AI-Ahmed · 2022-12-19T13:30:08Z

Sorry, @dniku, I have not noticed the script you wrote in the following comment – #512 – I want to point out that I added the notebook with the rendered output so that the student can see the results and understand the requirements so that they can understand the task. As shown in week_05_explore, the notebook there had rendered effects, which helped me understand the problem and the expected output.

Deleting the output results wouldn't be helpful, in my opinion, since the notebook itself is a bit hard for someone new to the topic – it took me eight weeks to figure out the case for both week_06 and week_08, only. Hence, it is better to keep the results in the notebook.

AI-Ahmed

I am sorry I have not notified you that I viewed that, too!

AI-Ahmed · 2023-03-10T00:39:13Z

Hello @dniku ,
I hope you are doing well. We have been four months, and we haven't checked that. Please, if you have time, let me know if there's anything else you need to modify.

Regards,

dniku · 2023-04-02T19:16:31Z

@AI-Ahmed

I have checked both week06_policy_based/a2c-optional.ipynb and week08_pomdp/practice_pytorch.ipynb in Colab — in particular, I have verified that both assignments are solvable end-to-end. That turned out to be not quite true because of some dependencies that have been updated since we originally wrote those notebooks. To fix those problems, I have filed two PRs:

[Week 06] Simplify Colab initialization and do some cleanup in the notebook #521 — for week06_policy_based/a2c-optional.ipynb
[Week 08] Update & clean up the notebook #522 — for week08_pomdp/practice_pytorch.ipynb

With regards to your PR, I'm afraid I will have to close it. Despite my comments, there are still many changes that I cannot merge, and it takes quite a lot of effort to go through review rounds. I would nevertheless like to thank you for your effort, and for drawing my attention to the fact that we had multiple problems with the assignments, and I have incorporated some of your changes into the PRs I linked above, like the use of the RecordVideo wrapper. Thanks — and if you feel like submitting any further PRs, please feel free to do so in the future.

P.S.: my implementation for week08_pomdp/practice_pytorch.ipynb trains without reward scaling, although it sometimes requires more iterations than 15k to cross the 10k reward threshold.

AI-Ahmed · 2023-04-02T20:10:58Z

First of all, @dniku, Thank you for mentioning me again. I agree; new things are happening in the community and are produced every day. Therefore, I am still open to my sub; I participated in this to improve what was written 2-4 years ago. Our goal is to have the most updated version to follow up with what is happening with the community.

I appreciate you guys working hard on this, and I have learnt from your Repos, so the least amount of appreciation will be participating in this great work.

Thank you again for opening two PRs for this issue.

Install & Update Packages Dependencies

09f923b

Install Specific version of Scipy Library to fix the error in Colab. Also, Intstalled GYM library for Atari & ROM Licence after downloading xvfb

AI-Ahmed added 6 commits November 12, 2022 11:21

Merge pull request #1 from yandexdataschool/master

792cb25

Merge with the base

Adding Video game dependencies

003ac4b

Download and install the video game dependencies, `ROM`, and install `xvfb` correctly. Downgrade the `gym` environment for monitoring the game.

Adding Video game dependencies

7a74c37

Download and install the video game dependencies, `ROM`, and install `xvfb` correctly. Downgrade the `gym` environment for monitoring the game.

Execute the installation

17c8cf6

Update a2c-optional.ipynb

b02a1c3

Merge pull request #2 from yandexdataschool/master

b435c93

Add the video lecture of Pavel Shvechikov (yandexdataschool#510)

dniku reviewed Nov 13, 2022

View reviewed changes

week06_policy_based/a2c-optional.ipynb Show resolved Hide resolved

week08_pomdp/practice_pytorch.ipynb Outdated Show resolved Hide resolved

week08_pomdp/practice_pytorch.ipynb Outdated Show resolved Hide resolved

AI-Ahmed added 2 commits November 13, 2022 19:54

Update the hash of the imported scripts

27ac2f4

Update the hash of the imported scripts of `atari_util.py` and `env_pool.py`.

fix the gym package version

e93455a

AI-Ahmed commented Nov 13, 2022

View reviewed changes

AI-Ahmed and others added 3 commits November 25, 2022 17:28

Update the notebook code workflow & libraries

1155d4e

Added new monitoring script and reward scaling for the reward for having better training performance with reducing the gradient exploration.

Merge branch 'yandexdataschool:master' into master

28b60fe

Simplify diff for week08_pomdp/practice_pytorch.ipynb

4af3160

AI-Ahmed commented Dec 4, 2022

View reviewed changes

dniku requested changes Dec 4, 2022

View reviewed changes

AI-Ahmed added 2 commits December 5, 2022 20:38

Update practice_pytorch.ipynb

69dbf40

Adjust the agent save & load method

ba24b7b

AI-Ahmed commented Dec 5, 2022

View reviewed changes

Merge branch 'master' into master

8915350

AI-Ahmed requested a review from dniku December 19, 2022 13:30

AI-Ahmed commented Feb 1, 2023

View reviewed changes

dniku mentioned this pull request Mar 26, 2023

[Week 06] Simplify Colab initialization and do some cleanup in the notebook #521

Merged

dniku closed this Apr 2, 2023

Install & Update Packages Dependencies #512

Install & Update Packages Dependencies #512

Conversation

AI-Ahmed commented Nov 11, 2022

review-notebook-app bot commented Nov 11, 2022

AI-Ahmed left a comment

Choose a reason for hiding this comment

AI-Ahmed left a comment

Choose a reason for hiding this comment

AI-Ahmed left a comment

Choose a reason for hiding this comment

dniku left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dniku commented Dec 4, 2022

AI-Ahmed left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AI-Ahmed commented Dec 19, 2022

AI-Ahmed left a comment

Choose a reason for hiding this comment

AI-Ahmed commented Mar 10, 2023

dniku commented Apr 2, 2023 • edited Loading

AI-Ahmed commented Apr 2, 2023

dniku commented Apr 2, 2023 •

edited

Loading