Install & Update Packages Dependencies

[AI-Ahmed]

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.

[review-notebook-app[bot]]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

[dniku]

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]

Sorry, @dniku, I have not noticed the script you wrote in the following comment – https://github.com/yandexdataschool/Practical_RL/pull/512/#issuecomment-1336510524 – 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]

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]

@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:

• https://github.com/yandexdataschool/Practical_RL/pull/521 — for week06_policy_based/a2c-optional.ipynb
• https://github.com/yandexdataschool/Practical_RL/pull/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]

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.