Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update notebooks using GPU & puzzle batching #35

Merged
merged 15 commits into from
Jul 17, 2024
Merged

Conversation

Xmaster6y
Copy link
Owner

@Xmaster6y Xmaster6y commented Jul 16, 2024

Summary by Sourcery

This pull request introduces GPU support for running models in the notebooks, updates notebook URLs to use underscores, enhances puzzle evaluation metrics, refactors Sampler classes, and updates documentation and tests accordingly.

  • New Features:
    • Added support for running models on GPU in the notebooks.
  • Enhancements:
    • Updated notebook links to use underscores instead of hyphens in URLs.
    • Improved puzzle evaluation metrics by adding normalized perplexity.
    • Refactored Sampler classes to remove use_argmax attribute from the base class and handle it in derived classes.
  • Documentation:
    • Updated README and notebook documentation to reflect new GPU support and updated URLs.
  • Tests:
    • Added new tests for puzzle evaluation with batch size.
    • Updated existing tests to reflect changes in Sampler class structure and new batch size handling.

Copy link
Contributor

sourcery-ai bot commented Jul 16, 2024

Reviewer's Guide by Sourcery

This pull request focuses on updating the notebooks to fix broken links, refactoring the sampling logic to improve flexibility and performance, and updating test cases to align with these changes. Additionally, it includes the removal of obsolete notebook files and the addition of new feature links in the README.

File-Level Changes

Files Changes
docs/source/notebooks/features/move_prediction.ipynb
docs/source/notebooks/features/convert_official_weights.ipynb
Fixed broken Colab and internal links to ensure proper navigation and execution.
src/lczerolens/play/sampling.py
src/lczerolens/play/puzzle.py
Refactored sampling and puzzle evaluation logic to improve flexibility and performance, including support for additional parameters and normalized perplexity calculation.
tests/unit/core/test_puzzle.py
tests/unit/core/test_sampling.py
Updated test cases to align with refactored sampling logic and added new test cases for batch size evaluation.
README.md
docs/source/notebooks/features/evaluate-models-on-puzzles.ipynb
docs/source/notebooks/features/run-models-on-gpu.ipynb
Fixed broken links in README and removed obsolete notebook files.

Tips
  • Trigger a new Sourcery review by commenting @sourcery-ai review on the pull request.
  • Continue your discussion with Sourcery by replying directly to review comments.
  • You can change your review settings at any time by accessing your dashboard:
    • Enable or disable the Sourcery-generated pull request summary or reviewer's guide;
    • Change the review language;
  • You can always contact us if you have any questions or feedback.

Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @Xmaster6y - I've reviewed your changes and found some issues that need to be addressed.

Blocking issues:

  • Hard-coded GitHub repository URL found. (link)
  • Hard-coded Google Drive file ID found. (link)
Here's what I looked at during the review
  • 🟡 General issues: 8 issues found
  • 🔴 Security: 2 blocking issues
  • 🟡 Testing: 2 issues found
  • 🟢 Complexity: all looks good
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.

src/lczerolens/play/sampling.py Outdated Show resolved Hide resolved
src/lczerolens/play/sampling.py Show resolved Hide resolved
tests/unit/core/test_puzzle.py Show resolved Hide resolved
tests/unit/core/test_sampling.py Show resolved Hide resolved
docs/source/notebooks/features/run_models_on_gpu.ipynb Outdated Show resolved Hide resolved
src/lczerolens/play/sampling.py Outdated Show resolved Hide resolved
@Xmaster6y Xmaster6y merged commit ef848f2 into main Jul 17, 2024
2 checks passed
@Xmaster6y Xmaster6y deleted the update-notebooks branch July 17, 2024 07:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant