You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mahout.apache.org by "viiccwen (via GitHub)" <gi...@apache.org> on 2026/04/11 07:38:09 UTC
[PR] feat: add IQP / IQP-Z latency and throughput benchmark [mahout]
viiccwen opened a new pull request, #1257:
URL: https://github.com/apache/mahout/pull/1257
### Related Issues
Closes #1256
### Changes
- [ ] Bug fix
- [x] New feature
- [ ] Refactoring
- [x] Documentation
- [x] Test
- [ ] CI/CD pipeline
- [ ] Other
### Why
QDP already supports `iqp` and `iqp-z` in the encoding engine, but the standard latency / throughput benchmark path did not. The Rust synthetic pipeline still treated those runs like amplitude-shaped inputs, and the Python benchmark layer did not expose or document the IQP-family options clearly.
This meant users could not benchmark IQP-family encodings through the same benchmark API/scripts used for amplitude.
### How
- updated `qdp-core` synthetic benchmark sizing so:
- `iqp-z` uses `num_qubits`
- `iqp` uses `num_qubits + num_qubits * (num_qubits - 1) / 2`
- generated deterministic IQP-family phase parameters in the Rust and Python synthetic benchmark helpers
- added `iqp` / `iqp-z` support to:
- `benchmark_latency.py`
- `benchmark_throughput.py`
- `benchmark_pytorch_ref.py`
- extended the PyTorch fallback path to accept `iqp-z`
- added Rust and Python tests covering:
- pipeline vector sizing
- deterministic IQP-family sample generation
- fallback benchmark execution
- updated benchmark and Python API documentation
**NOTICED: Due to #1252, we only can testing Mahout IQP/IQP-Z, using TODO label and warning to fix it in the future.**
## Checklist
- [x] Added or updated unit tests for all changes
- [x] Added or updated documentation for all changes
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mahout.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add IQP / IQP-Z latency and throughput benchmark [mahout]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #1257:
URL: https://github.com/apache/mahout/pull/1257#discussion_r3190755682
##########
qdp/qdp-python/benchmark/benchmark_throughput.py:
##########
@@ -170,6 +170,16 @@ def run_mahout(
return result.duration_sec, result.vectors_per_sec
+def _sample_dim(num_qubits: int, encoding_method: str) -> int:
+ if encoding_method == "basis":
+ return 1
+ if encoding_method in {"angle", "iqp-z"}:
+ return num_qubits
+ if encoding_method == "iqp":
+ return num_qubits + num_qubits * (num_qubits - 1) // 2
+ return 1 << num_qubits
Review Comment:
`basis` currently returns `1`, but this script’s benchmark utilities treat the passed dimension as the *upper bound* for the basis index generation (i.e., should be `2**num_qubits`). With `1`, synthetic basis inputs will be all zeros, producing incorrect/non-representative benchmarks. Fix by returning `1 << num_qubits` for `basis` here (and keep `angle/iqp/iqp-z` as parameter counts).
##########
qdp/qdp-python/benchmark/benchmark_latency.py:
##########
@@ -113,6 +113,16 @@ def run_mahout(
return result.duration_sec, result.latency_ms_per_vector
+def _sample_dim(num_qubits: int, encoding_method: str) -> int:
+ if encoding_method == "basis":
+ return 1
+ if encoding_method in {"angle", "iqp-z"}:
+ return num_qubits
+ if encoding_method == "iqp":
+ return num_qubits + num_qubits * (num_qubits - 1) // 2
+ return 1 << num_qubits
Review Comment:
Same issue as the throughput benchmark: returning `1` for `basis` causes synthetic basis indices to be generated in `[0, 1)`, i.e., always `0`. Update `basis` to return `1 << num_qubits` to preserve correct basis index ranges.
##########
qdp/qdp-python/benchmark/README.md:
##########
@@ -87,7 +87,7 @@ Notes:
- `--frameworks` is a comma-separated list or `all`.
Options: `mahout`, `pennylane`, `qiskit-init`, `qiskit-statevector`.
-- `--encoding-method` selects the encoding method: `amplitude` (default) or `basis`.
+- `--encoding-method` selects the encoding method: `amplitude` (default), `angle`, `basis`, `iqp`, or `iqp-z`.
- The latency test reports average milliseconds per vector.
- Flags:
- `--qubits`: controls vector length (`2^qubits`).
Review Comment:
This statement is no longer correct now that `--encoding-method` includes `angle`, `iqp`, and `iqp-z` (where the input length is `num_qubits` or `n + n(n-1)/2`). Update this line to describe that input length depends on the selected encoding method (and specify the formulas).
##########
qdp/qdp-python/qumat_qdp/torch_ref.py:
##########
@@ -337,24 +337,30 @@ def encode(
encoding_method: str = "amplitude",
*,
device: torch.device | str | None = None,
- **kwargs: object,
+ enable_zz: bool = True,
) -> torch.Tensor:
"""Dispatch to the appropriate encoding function by method name.
Args:
data: Input tensor.
num_qubits: Number of qubits.
- encoding_method: One of ``"amplitude"``, ``"angle"``, ``"basis"``, ``"iqp"``.
+ encoding_method: One of ``"amplitude"``, ``"angle"``, ``"basis"``, ``"iqp"``, ``"iqp-z"``.
device: Target device.
- **kwargs: Extra arguments forwarded to the encoder (e.g. *enable_zz* for IQP).
+ enable_zz: Whether IQP encoding includes ZZ interaction terms. Ignored for
+ non-IQP encodings. ``"iqp-z"`` always forces this to ``False``.
Returns:
Complex tensor of shape ``(batch, 2**num_qubits)``.
"""
+ if encoding_method == "iqp-z":
+ return iqp_encode(data, num_qubits, device=device, enable_zz=False)
+ if encoding_method == "iqp":
+ return iqp_encode(data, num_qubits, device=device, enable_zz=enable_zz)
+
fn = _ENCODERS.get(encoding_method)
if fn is None:
raise ValueError(
f"Unknown encoding method {encoding_method!r}. "
f"Supported: {', '.join(sorted(_ENCODERS))}"
)
Review Comment:
The error message builds its supported-method list from `_ENCODERS`, but `iqp-z` is handled via a special-case branch and may not appear in `_ENCODERS`. This can produce a misleading 'Supported:' list. Consider including special-cased methods (e.g., `iqp`/`iqp-z`) in the displayed supported set, or building the list from a single source of truth.
##########
qdp/qdp-python/qumat_qdp/torch_ref.py:
##########
@@ -337,24 +337,30 @@ def encode(
encoding_method: str = "amplitude",
*,
device: torch.device | str | None = None,
- **kwargs: object,
+ enable_zz: bool = True,
) -> torch.Tensor:
Review Comment:
This change removes `**kwargs` from `encode()`, which can be a breaking API change for any downstream callers passing extra encoder-specific options. If backward compatibility is required, consider keeping `**kwargs` and either (a) extracting `enable_zz` from it with a deprecation path, or (b) explicitly rejecting unknown kwargs with a clear error message.
##########
qdp/qdp-core/src/pipeline_runner.rs:
##########
@@ -537,11 +537,13 @@ pub fn vector_len(num_qubits: u32, encoding_method: &str) -> usize {
match encoding_method.to_lowercase().as_str() {
"angle" => n,
"basis" => 1,
+ "iqp-z" => n,
+ "iqp" => n + n.saturating_mul(n.saturating_sub(1)) / 2,
Review Comment:
The IQP length computation uses `saturating_mul`, but the final `n + (...)` can still overflow `usize` for large `n` (wrapping in release, panicking in debug depending on settings). Use `saturating_add` (or a checked add with a clear error) to keep overflow behavior consistent with the rest of the expression.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mahout.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add IQP / IQP-Z latency and throughput benchmark [mahout]
Posted by "viiccwen (via GitHub)" <gi...@apache.org>.
viiccwen commented on code in PR #1257:
URL: https://github.com/apache/mahout/pull/1257#discussion_r3195833044
##########
qdp/qdp-python/benchmark/README.md:
##########
@@ -87,7 +87,7 @@ Notes:
- `--frameworks` is a comma-separated list or `all`.
Options: `mahout`, `pennylane`, `qiskit-init`, `qiskit-statevector`.
-- `--encoding-method` selects the encoding method: `amplitude` (default) or `basis`.
+- `--encoding-method` selects the encoding method: `amplitude` (default), `angle`, `basis`, `iqp`, or `iqp-z`.
- The latency test reports average milliseconds per vector.
- Flags:
- `--qubits`: controls vector length (`2^qubits`).
Review Comment:
Updated the benchmark README to describe the input-size formulas per encoding method. Basis now documents the single-index input shape, while amplitude/angle/iqp/iqp-z list their respective sample lengths. Fixed in 986683879.
##########
qdp/qdp-python/qumat_qdp/torch_ref.py:
##########
@@ -337,24 +337,30 @@ def encode(
encoding_method: str = "amplitude",
*,
device: torch.device | str | None = None,
- **kwargs: object,
+ enable_zz: bool = True,
) -> torch.Tensor:
"""Dispatch to the appropriate encoding function by method name.
Args:
data: Input tensor.
num_qubits: Number of qubits.
- encoding_method: One of ``"amplitude"``, ``"angle"``, ``"basis"``, ``"iqp"``.
+ encoding_method: One of ``"amplitude"``, ``"angle"``, ``"basis"``, ``"iqp"``, ``"iqp-z"``.
device: Target device.
- **kwargs: Extra arguments forwarded to the encoder (e.g. *enable_zz* for IQP).
+ enable_zz: Whether IQP encoding includes ZZ interaction terms. Ignored for
+ non-IQP encodings. ``"iqp-z"`` always forces this to ``False``.
Returns:
Complex tensor of shape ``(batch, 2**num_qubits)``.
"""
+ if encoding_method == "iqp-z":
+ return iqp_encode(data, num_qubits, device=device, enable_zz=False)
+ if encoding_method == "iqp":
+ return iqp_encode(data, num_qubits, device=device, enable_zz=enable_zz)
+
fn = _ENCODERS.get(encoding_method)
if fn is None:
raise ValueError(
f"Unknown encoding method {encoding_method!r}. "
f"Supported: {', '.join(sorted(_ENCODERS))}"
)
Review Comment:
Adjusted the torch_ref dispatcher error path so the supported-method list includes the iqp-z special case as well. Added a regression test for the message. Fixed in 986683879.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mahout.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add IQP / IQP-Z latency and throughput benchmark [mahout]
Posted by "400Ping (via GitHub)" <gi...@apache.org>.
400Ping commented on PR #1257:
URL: https://github.com/apache/mahout/pull/1257#issuecomment-4230994257
Note that I only did a quick scan for this PR, a little busy this week
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mahout.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add IQP / IQP-Z latency and throughput benchmark [mahout]
Posted by "ryankert01 (via GitHub)" <gi...@apache.org>.
ryankert01 merged PR #1257:
URL: https://github.com/apache/mahout/pull/1257
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mahout.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org