Skip to content

[QDP] Add encoding method parameter to all benchmarks #840

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
Merged
guan404ming merged 2 commits into from
Jan 17, 2026
Merged

[QDP] Add encoding method parameter to all benchmarks #840

guan404ming merged 2 commits into from
Jan 17, 2026
+379 −164

Conversation

@guan404ming
Copy link
Member

@guan404ming guan404ming commented Jan 16, 2026 •
edited
Loading

Purpose of PR

  • Supports amplitude (default) and basis encoding methods in benchmark
  • All 5 benchmark scripts updated: latency, throughput, e2e, numpy_io, latency_pytorch (e2e needs a follow to support streaming)

Related Issues or PRs

Changes Made

  • Bug fix
  • New feature
  • Refactoring
  • Documentation
  • Test
  • CI/CD pipeline
  • Other

Breaking Changes

  • Yes
  • No

Checklist

  • Added or updated unit tests for all changes
  • Added or updated documentation for all changes
  • Successfully built and ran all unit tests or manual tests locally
  • PR title follows "MAHOUT-XXX: Brief Description" format (if related to an issue)
  • Code follows ASF guidelines

ryankert01
ryankert01 reviewed Jan 16, 2026
View reviewed changes
qdp/qdp-python/benchmark/benchmark_e2e.py Outdated
all_data = np.random.randint(0, dim, size=(n_samples, 1)).astype(np.float64)
else:
# Amplitude encoding: full vectors
all_data = np.random.rand(n_samples, dim).astype(np.float64)
Copy link
Member

@ryankert01 ryankert01 Jan 16, 2026

Choose a reason for hiding this comment

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

Can this be a shared utility function that can be used on multiple benchmark?

Copy link
Member Author

@guan404ming guan404ming Jan 16, 2026

Choose a reason for hiding this comment

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

Sure, I've updated.

@guan404ming guan404ming force-pushed the add-encoding-method-param-benchmarks branch from 5211a2b to 6c9118c Compare January 16, 2026 14:01
@guan404ming
Copy link
Member Author

guan404ming commented Jan 17, 2026

cc @400Ping

@rich7420
Copy link
Contributor

rich7420 commented Jan 17, 2026

Thanks for the patch @guan404ming !
I think we should update qdp/qdp-python/benchmark/README.md as well. I'll try run all benchmarks later.

@guan404ming guan404ming force-pushed the add-encoding-method-param-benchmarks branch from fcd6999 to 1f53d69 Compare January 17, 2026 14:03
@guan404ming
Copy link
Member Author

guan404ming commented Jan 17, 2026

I think we should update qdp/qdp-python/benchmark/README.md as well. I'll try run all benchmarks later.

Nice catch, I've updated. Thanks!

ryankert01
ryankert01 approved these changes Jan 17, 2026
View reviewed changes
Copy link
Member

@ryankert01 ryankert01 left a comment

Choose a reason for hiding this comment

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

Overall lg, cc @rich7420 @400Ping

400Ping
400Ping approved these changes Jan 17, 2026
View reviewed changes
Copy link
Contributor

@400Ping 400Ping left a comment

Choose a reason for hiding this comment

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

Overall LGTM

@guan404ming
Copy link
Member Author

guan404ming commented Jan 17, 2026

Going to merge this. Feel free to open pr to refine this. Thanks!

@guan404ming guan404ming merged commit ccfc116 into apache:main Jan 17, 2026
4 checks passed
@guan404ming guan404ming deleted the add-encoding-method-param-benchmarks branch January 17, 2026 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ryankert01 ryankert01 ryankert01 approved these changes

@rich7420 rich7420 Awaiting requested review from rich7420

+1 more reviewer

@400Ping 400Ping 400Ping approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

qdp

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@guan404ming @rich7420 @400Ping @ryankert01 @rawkintrevo