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

Pr 36 #37

Merged
merged 17 commits into from
Oct 18, 2024
Merged

Pr 36 #37

merged 17 commits into from
Oct 18, 2024

Conversation

ternaus
Copy link
Contributor

@ternaus ternaus commented Oct 18, 2024

Summary by Sourcery

Enhance the benchmarking framework by introducing a new script for comparing LUT implementations, refactor the BenchmarkTest class for better extensibility, and add support for in-place operations in image processing functions. Update documentation and tests to reflect these changes, and expand CI coverage to include newer Python versions.

New Features:

  • Introduce a new benchmarking script for comparing StringZilla's LUT implementation with OpenCV's cv2.LUT, including detailed performance metrics and speedup calculations.

Enhancements:

  • Refactor the BenchmarkTest class to use abstract methods for transform functions, improving the extensibility of the benchmarking framework.
  • Add inplace parameter to various image processing functions to allow in-place operations, enhancing performance flexibility.

Build:

  • Update setup.py to include stringzilla as a dependency, ensuring compatibility with new benchmarking features.

CI:

  • Expand CI testing matrix to include Python versions 3.9 to 3.13, ensuring broader compatibility and future-proofing.

Documentation:

  • Revise README.md to provide a more comprehensive overview of Albucore's features, usage, and performance benchmarks.
  • Add detailed README files for new benchmark suites, explaining their purpose, requirements, and usage.

Tests:

  • Add new tests for the sz_lut function to ensure its correctness and performance against cv2.LUT.
  • Refactor existing tests to use np.testing.assert_array_equal and np.testing.assert_allclose for more robust and informative assertions.

ashvardanian and others added 14 commits October 13, 2024 01:55
StringZilla brings hardware-accelerated
Look-Up Table transformations that can
leverage AVX-512 VBMI instructions on
recent Intel Ice Lake CPUs (installed in
most DGX servers), as well as older
Intel Haswell, and newer Arm CPUs,
like AWS Graviton 4.

Preliminary benchmarks on new x86 CPUs
suggest up to 4x performance improvements
compared to the OpenCV baselines.
Copy link
Contributor

sourcery-ai bot commented Oct 18, 2024

Reviewer's Guide by Sourcery

This pull request implements significant changes to the Albucore project, including refactoring of benchmark code, addition of new benchmarks, updates to existing tests, and modifications to project configuration files. The changes aim to improve performance, add new features, and enhance the overall structure of the project.

Architecture diagram for CI/CD pipeline changes

graph TD;
    A[CI/CD Pipeline] --> B[Python 3.9]
    A --> C[Python 3.10]
    A --> D[Python 3.11]
    A --> E[Python 3.12]
    A --> F[Python 3.13]
    B --> G[Checkout]
    C --> G
    D --> G
    E --> G
    F --> G
    G --> H[Install Dependencies]
    H --> I[Run Checks]
    I --> J[Run Tests]
Loading

Updated class diagram for BenchmarkTest

classDiagram
    class BenchmarkTest {
        <<abstract>>
        - num_channels: int
        - img_type: None
        + albucore_transform(img: np.ndarray) np.ndarray
        + opencv_transform(img: np.ndarray) np.ndarray
        + numpy_transform(img: np.ndarray) np.ndarray
        + lut_transform(img: np.ndarray) np.ndarray
        + torchvision_transform(img: torch.Tensor) torch.Tensor
        + __str__() str
    }
Loading

Updated class diagram for apply_lut function

classDiagram
    class Functions {
        + create_lut_array(dtype: type[np.number], value: float | np.ndarray, operation: Literal["add", "multiply", "power"]) np.ndarray
        + sz_lut(img: np.ndarray, lut: np.ndarray, inplace: bool) np.ndarray
        + apply_lut(img: np.ndarray, value: float | np.ndarray, operation: Literal["add", "multiply", "power"], inplace: bool) np.ndarray
    }
Loading

File-Level Changes

Change Details Files
Refactored and enhanced the benchmark system
  • Added new benchmark for comparing StringZilla LUT with cv2.LUT
  • Updated existing benchmark code for better performance and flexibility
  • Added README files for benchmark directories
  • Removed old benchmark result files
benchmark/benchmark.py
benchmark/stringzilla_vs_cv2_lut/benchmark_lut.py
benchmark/stringzilla_vs_cv2_lut/README.md
benchmark/albucore_benchmark/README.md
benchmark/README.md
Updated core functionality in albucore/functions.py
  • Added new sz_lut function using StringZilla
  • Modified existing functions to support inplace operations
  • Updated function signatures and type hints
albucore/functions.py
Updated and added new tests
  • Modified existing tests to use np.testing assertions
  • Added new test for LUT functionality
  • Updated test parameters and error handling
tests/test_add.py
tests/test_normalize.py
tests/test_add_weighted.py
tests/test_multiply.py
tests/test_multiply_add.py
tests/test_normalize_per_image.py
tests/test_power.py
tests/test_utils.py
tests/test_lut.py
Updated project configuration and CI/CD
  • Updated .pre-commit-config.yaml with new versions and hooks
  • Modified CI workflow to run tests on multiple Python versions
  • Updated setup.py with new dependencies and Python version support
.pre-commit-config.yaml
.github/workflows/ci.yml
.github/workflows/upload_to_pypi.yml
setup.py
Updated project documentation
  • Significantly revised README.md with new project description and features
  • Updated benchmark.sh script
README.md
benchmark.sh

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time. You can also use
    this command to specify where the summary should be inserted.

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@codecov-commenter
Copy link

codecov-commenter commented Oct 18, 2024

Codecov Report

Attention: Patch coverage is 46.37681% with 148 lines in your changes missing coverage. Please review.

Please upload report for BASE (main@05d0d48). Learn more about missing BASE report.

Files with missing lines Patch % Lines
benchmark/stringzilla_vs_cv2_lut/benchmark_lut.py 0.00% 75 Missing ⚠️
benchmark/albucore_benchmark/benchmark.py 0.00% 48 Missing ⚠️
benchmark/utils.py 0.00% 17 Missing ⚠️
tests/test_normalize_per_image.py 55.55% 4 Missing ⚠️
tests/test_normalize.py 76.92% 3 Missing ⚠️
albucore/functions.py 97.22% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main      #37   +/-   ##
=======================================
  Coverage        ?   59.55%           
=======================================
  Files           ?       19           
  Lines           ?     2119           
  Branches        ?        0           
=======================================
  Hits            ?     1262           
  Misses          ?      857           
  Partials        ?        0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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 @ternaus - I've reviewed your changes - here's some feedback:

Overall Comments:

  • Consider archiving or finding an alternative storage solution for the removed benchmark result files to preserve historical data.
Here's what I looked at during the review
  • 🟢 General issues: all looks good
  • 🟢 Security: all looks good
  • 🟡 Testing: 5 issues found
  • 🟡 Complexity: 1 issue found
  • 🟡 Documentation: 1 issue found

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 and I'll use the feedback to improve your reviews.

tests/test_add.py Show resolved Hide resolved
tests/test_normalize.py Show resolved Hide resolved
tests/test_normalize_per_image.py Show resolved Hide resolved
tests/test_lut.py Outdated Show resolved Hide resolved
tests/test_lut.py Show resolved Hide resolved
tests/test_power.py Show resolved Hide resolved
tests/test_power.py Show resolved Hide resolved
tests/test_power.py Show resolved Hide resolved
tests/test_utils.py Show resolved Hide resolved
benchmark/utils.py Show resolved Hide resolved
@ternaus ternaus merged commit 1624105 into main Oct 18, 2024
19 checks passed
@ternaus ternaus deleted the pr-36 branch October 18, 2024 01:57
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.

3 participants