yandexdataschool / Practical_RL

A course in reinforcement learning in the wild
The Unlicense
5.93k stars 1.7k forks source link

Approx_RL main notebook update #544

Open IsaevRustam opened 3 weeks ago

IsaevRustam commented 3 weeks ago

Changes Made

  1. Refactored Wrappers

    • Replaced the deprecated gymnasium.wrappers.frame_stack with gym.wrappers.stateful_observation.FrameStackObservation to resolve the following error:
      module 'gymnasium.wrappers' has no attribute 'frame_stack'
    • Ensures compatibility with the latest Gymnasium version.
  2. Fixed Image Display Issues

    • Updated image handling logic to ensure proper display of all images.
  3. Added Flexible Logging with logger.py

    • Introduced logger.py with the Logger class, providing options to log to TensorBoard or use matplotlib.pyplot (plt) for visualization.

    • Usage Example:

      from logger import Logger
      
      # To log to TensorBoard
      logger = Logger(use_tensorboard=True, log_dir='logs/')
      
      # To log using matplotlib
      logger = Logger(use_tensorboard=False)
      
      logger.log_loss(loss_value, step)
      logger.log_grad_norm(grad_norm_value, step)
      logger.log_mean_reward(mean_reward_value, step)
      logger.log_initial_state_v(initial_v_value, step)
      
      # If using matplotlib for logging
      logger.plot()
      
      # Close the logger
      logger.close()

Notes

review-notebook-app[bot] commented 3 weeks ago

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

dniku commented 3 weeks ago

Thanks for the PR.

I would appreciate a detailed description if it wasn't hallucinated. E.g. (1) ("Upgraded all project dependencies to their latest versions") is definitely not in this PR.

Please update the PR description so it corresponds to reality. Deleting that auto-generated summary is fine.

IsaevRustam commented 3 weeks ago

Deleting that auto-generated summary is fine.

Thank you for the response. After reviewing it with fresh eyes, I realized there were no updates to libraries or reqs dependencies in the final PR.

I was dealing with an error related to the ALE namespace and was solving dependencies problems with this issue. I was solving deps errors and so on with gym and gymnasium (with ALE namespace) in different branch, but after few evenings I found a simple solution:

gym.register_envs(ale_py)

This caused the cursor bot to include those changes in the summary, even though they weren’t part of the final PR. My bad, didn't check it properly.

P.S. Updated PR description. Thanks for remarks.

IsaevRustam commented 1 week ago

Hey!

Just recognized about this PR and its not closed or merged yet.

Should I close this PR or mb draft it?