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.

Sign up for GitHub

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

Jump to bottom

Feature/benchmarking #802

Merged
merged 85 commits into from
Nov 28, 2024
Merged

Feature/benchmarking #802

merged 85 commits into from
Nov 28, 2024

Conversation

qh681248
Copy link
Contributor

@qh681248 qh681248 commented Oct 9, 2024 •
edited
Loading

PR Type

  • Feature

Description

Added two files mnist_benchmark.py and blobs_benchmark.py

How Has This Been Tested?

Test A: (Write your answer here.)
Test B: (Write your answer here.)
Test C: (Write your answer here.)

Does this PR introduce a breaking change?

(Write your answer here.)

Screenshots

(Write your answer here.)

Checklist before requesting a review

  • I have made sure that my PR is not a duplicate.
  • My code follows the style guidelines of this project.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have performed a self-review of my code.
  • I have made corresponding changes to the documentation.
  • My changes generate no new warnings.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.
  • Any dependent changes have been merged and published in downstream modules.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 9, 2024

Performance review

Commit 8a9d459 - Merge 1dffbc6 into 7aa60a7

No significant changes to performance.

@pc532627 pc532627 mentioned this pull request Oct 9, 2024
Merged
9 tasks
@qh681248 qh681248 linked an issue Oct 10, 2024 that may be closed by this pull request
Coreset algorithm benchmarks for different data types #509
Closed
@github-actions GitHub Actions
Copy link
Contributor

Performance review

Commit c281065 - Merge 055206b into 1832579

No significant changes to performance.

@github-actions GitHub Actions
Copy link
Contributor

Performance review

Commit 91933db - Merge 78cd2e0 into 1832579

No significant changes to performance.

@github-actions GitHub Actions
Copy link
Contributor

Performance review

Commit 4be645f - Merge 9f799bf into 1832579

No significant changes to performance.

tp832944
tp832944 previously requested changes Oct 10, 2024
View reviewed changes
Copy link
Contributor

@tp832944 tp832944 left a comment

Choose a reason for hiding this comment

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

I haven't got my head around everything yet, but this is good progress and you're clear to continue the job on benchmarking. I may have chucked on quite a few comments, although nothing is major.

CHANGELOG.md Outdated Show resolved Hide resolved
benchmark/blobs_benchmark.py Outdated Show resolved Hide resolved
benchmark/blobs_benchmark.py Outdated Show resolved Hide resolved
benchmark/blobs_benchmark.py Outdated Show resolved Hide resolved
benchmark/blobs_benchmark.py Outdated Show resolved Hide resolved
benchmark/mnist_benchmark_visualiser.py Outdated Show resolved Hide resolved
benchmark/mnist_benchmark_visualiser.py Show resolved Hide resolved
benchmark/blobs_benchmark.py Show resolved Hide resolved
benchmark/mnist_benchmark.py Outdated Show resolved Hide resolved
benchmark/mnist_benchmark.py Show resolved Hide resolved
@github-actions GitHub Actions
Copy link
Contributor

Performance review

Commit efb2040 - Merge b84ff03 into 1832579

No significant changes to performance.

@github-actions GitHub Actions
Copy link
Contributor

Performance review

Commit 1d62686 - Merge 41f4f09 into 1832579

No significant changes to performance.

@github-actions GitHub Actions
Copy link
Contributor

Performance review

Commit e1f9141 - Merge 5d0a8e2 into 3463539

Statistically significant changes

  • basic_rpc:
    • OLD: compilation 0.6253 units ± 0.03252 units; execution 0.142 units ± 0.001823 units
    • NEW: compilation 0.6087 units ± 0.02183 units; execution 0.1634 units ± 0.00134 units
    • Significant increase in execution time (15.05%, p=8.125e-16)

Normalisation values for new data:
Compilation: 1 unit = 375.27 ms
Execution: 1 unit = 795.81 ms

@github-actions GitHub Actions
Copy link
Contributor

Performance review

Commit 63937b4 - Merge 4180f52 into 3463539

Statistically significant changes

  • basic_rpc:
    • OLD: compilation 0.6253 units ± 0.03252 units; execution 0.142 units ± 0.001823 units
    • NEW: compilation 0.6025 units ± 0.03407 units; execution 0.1573 units ± 0.0005494 units
    • Significant increase in execution time (10.72%, p=7.818e-11)

Normalisation values for new data:
Compilation: 1 unit = 372.3 ms
Execution: 1 unit = 793.52 ms

@github-actions GitHub Actions
Copy link
Contributor

Performance review

Commit 65b253f - Merge db3d3ee into 3463539

No significant changes to performance.

@github-actions GitHub Actions
Copy link
Contributor

Performance review

Commit 88742a6 - Merge b912461 into 3463539

Statistically significant changes

  • basic_rpc:
    • OLD: compilation 0.6253 units ± 0.03252 units; execution 0.142 units ± 0.001823 units
    • NEW: compilation 0.6208 units ± 0.02652 units; execution 0.1588 units ± 0.0007635 units
    • Significant increase in execution time (11.78%, p=4.115e-12)

Normalisation values for new data:
Compilation: 1 unit = 367.39 ms
Execution: 1 unit = 792.56 ms

@github-actions GitHub Actions
Copy link
Contributor

Performance review

Commit f042a4a - Merge cb5181c into 3556e02

No significant changes to performance.

bk958178
bk958178 requested changes Nov 26, 2024
View reviewed changes
Copy link
Contributor

@bk958178 bk958178 left a comment

Choose a reason for hiding this comment

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

@qh681248 A small handful of further changes requested. Thanks in advance for your response

benchmark/mnist_benchmark_coresets_only.py Outdated Show resolved Hide resolved
benchmark/mnist_benchmark_coresets_only.py Outdated Show resolved Hide resolved
benchmark/mnist_benchmark_coresets_only.py Outdated Show resolved Hide resolved
benchmark/mnist_benchmark_coresets_only.py Outdated Show resolved Hide resolved
@github-actions GitHub Actions
Copy link
Contributor

Performance review

Commit 1daca62 - Merge b3d9d8d into ed78130

No significant changes to performance.

@github-actions GitHub Actions
Copy link
Contributor

Performance review

Commit 37318e2 - Merge acb252b into ed78130

No significant changes to performance.

@qh681248 qh681248 force-pushed the feature/benchmarking branch from acb252b to eaf1dd4 Compare November 27, 2024 13:05
@github-actions GitHub Actions
Copy link
Contributor

Performance review

Commit b7223c7 - Merge be7cf4f into ed78130

No significant changes to performance.

@github-actions GitHub Actions
Copy link
Contributor

Performance review

Commit bacf762 - Merge d92880d into ed78130

No significant changes to performance.

bk958178
bk958178 approved these changes Nov 28, 2024
View reviewed changes
Copy link
Contributor

@bk958178 bk958178 left a comment

Choose a reason for hiding this comment

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

Great work, @qh681248. Approving and merging

@bk958178 bk958178 dismissed tp832944’s stale review November 28, 2024 13:58

Dismissing outdated review

@bk958178 bk958178 merged commit 5039cff into main Nov 28, 2024
19 checks passed
@bk958178 bk958178 deleted the feature/benchmarking branch November 28, 2024 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bk958178 bk958178 bk958178 approved these changes

@tp832944 tp832944 tp832944 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Coreset algorithm benchmarks for different data types
3 participants
@qh681248 @bk958178 @tp832944