feat: Add conversion between `jeff` and Qiskit by kawacukennedy · Pull Request #64 · unitaryfoundation/jeff

kawacukennedy · 2026-06-04T18:02:51Z

Implements the conversion between jeff binary format and Qiskit QuantumCircuit using Qiskit's C API (qiskit.h) and capnp C++ API. Closes #60.

Conversion directions

jeff → Qiskit (jeff-qiskit-convert read): Reads a jeff binary, builds a QuantumCircuit via Qiskit C API.
Qiskit → jeff (jeff-qiskit-convert write): Reads a QuantumCircuit via Qiskit C API, writes a jeff binary via capnp C++ builder.
Round-trip (jeff-qiskit-convert test): Builds a circuit, writes jeff, reads back, and verifies instruction count.

Gate coverage

Supports 50+ Qiskit gates via a data-driven GateInfo table covering well-known gates, multi-controlled gates (CX, CCX, C3X, etc.), and custom gates via the jeff string table.

Build & usage

cmake -B build tools/qiskit_convert
cmake --build build
./build/jeff-qiskit-convert test

# jeff → Qiskit
./build/jeff-qiskit-convert read circuit.jeff [diagram.txt]

# Qiskit → jeff (test circuit, 2 qubits 2 clbits by default)
./build/jeff-qiskit-convert write output.jeff [n_qubits n_clbits]

Qiskit C API path is auto-discovered via Python; set QISKIT_ROOT env var if needed.

Tests

pytest tools/qiskit_convert/tests/

Known limitations

Gate parameters are placeholder 0.0 (not wired from Qiskit C API InstBuf.params yet)
Straight-line quantum programs only (no loops, conditionals, sub-function calls)
Tested on macOS (AppleClang 17, capnp 1.3.0, Qiskit 2.4.1)

Implements conversion between jeff binary format and Qiskit QuantumCircuit using the Qiskit C API for circuit building as specified in #60. Supports 50+ standard Qiskit gates and passes 10 round-trip tests verified against both Python operation counts and direct C API readback.

denialhaag · 2026-06-04T18:11:37Z

Thanks for your interest in contributing to jeff, @kawacukennedy!

The issue description does not call for an implementation in Python. Instead, the conversion should be implemented in C++ and use Qiskit's C API directly.

denialhaag

Thanks for porting the translation over to C++! 🙂

I briefly scanned the diff and left a few comments; you can find them below.

More importantly, the conversion is currently not being tested. We need unit tests that can run in our CI.

…capnp, CI tests

…view comment

…ailure)

codecov-commenter · 2026-06-06T19:07:17Z

Codecov Report

❌ Patch coverage is 28.00000% with 36 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (main@5559ee1). Learn more about missing BASE report.

Files with missing lines	Patch %	Lines
tools/qiskit_convert/tests/test_qiskit_convert.py	28.00%	36 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main      #64   +/-   ##
=======================================
  Coverage        ?   67.86%           
=======================================
  Files           ?        5           
  Lines           ?     1254           
  Branches        ?        0           
=======================================
  Hits            ?      851           
  Misses          ?      403           
  Partials        ?        0

Flag	Coverage Δ
python	`67.86% <28.00%> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

burgholzer · 2026-06-07T21:39:34Z

@denialhaag @burgholzer — gentle bump on this. All review items have been addressed across the latest commits (direct linking, data-driven gate table, build-time capnp generation, CI tests, formatting). CI is passing on all 10 non-skipped checks. Would appreciate a re-review when you have a moment. Thanks!

Please stop spamming the PR with comments and abide by the unitary hack rules.
Maintainers will get back to you in due time and provide you with feedback.

denialhaag

Thanks for your continued work on the Qiskit conversion, @kawacukennedy! 🙂

You can find some more feedback below. You will see that there are still quite a few things left to do before we can merge this.

denialhaag · 2026-06-07T22:27:57Z

+# Generate Cap'n Proto bindings from schema at build time
+set(CAPNPC_SRC_PREFIX ${CMAKE_CURRENT_SOURCE_DIR}/../../impl)
+set(CAPNPC_OUTPUT_DIR ${CMAKE_CURRENT_BINARY_DIR})
+set(CAPNPC_IMPORT_DIRS ${CAPNP_INCLUDE_DIRECTORY})
+capnp_generate_cpp(CAPNP_SRCS CAPNP_HDRS
+    ${CAPNPC_SRC_PREFIX}/capnp/jeff.capnp
+)


Is this really necessary? We should be able to use jeff.capnp.c++ and jeff.capnp.h.

denialhaag · 2026-06-07T22:31:01Z

+    if (cmd == "write") {
+        if (argc < 3) {
+            fprintf(stderr, "error: missing output path\n");
+            return 1;
+        }
+        uint32_t n_qubits = (argc > 3) ? (uint32_t)atoi(argv[3]) : 2;
+        uint32_t n_clbits = (argc > 4) ? (uint32_t)atoi(argv[4]) : 2;
+
+        QkCircuit *circuit = qk_circuit_new(n_qubits, n_clbits);
+        uint32_t q0[] = {0};
+        uint32_t q1[] = {1};
+        uint32_t q01[] = {0, 1};
+
+        qk_circuit_gate(circuit, QkGate_X, q0, nullptr);
+        qk_circuit_gate(circuit, QkGate_X, q1, nullptr);
+        qk_circuit_gate(circuit, QkGate_H, q0, nullptr);
+        qk_circuit_gate(circuit, QkGate_CX, q01, nullptr);
+        double ry_p[] = {M_PI / 4.0};
+        qk_circuit_gate(circuit, QkGate_RY, q1, ry_p);
+        qk_circuit_measure(circuit, 0, 0);
+        qk_circuit_measure(circuit, 1, 1);
+
+        if (qiskit_to_jeff(circuit, argv[2]) < 0) {
+            qk_circuit_free(circuit);
+            return 1;
+        }
+        printf("Wrote %s\n", argv[2]);
+        qk_circuit_free(circuit);
+        return 0;
+    }
+
+    if (cmd == "test") {
+        printf("Running round-trip verification...\n");
+
+        QkCircuit *circuit = qk_circuit_new(2, 2);
+        uint32_t q0[] = {0};
+        uint32_t q1[] = {1};
+        uint32_t q01[] = {0, 1};
+
+        qk_circuit_gate(circuit, QkGate_X, q0, nullptr);
+        qk_circuit_gate(circuit, QkGate_H, q1, nullptr);
+        qk_circuit_gate(circuit, QkGate_CX, q01, nullptr);
+        double ry_p[] = {-2.0 * M_PI / 3.0};
+        qk_circuit_gate(circuit, QkGate_RY, q1, ry_p);
+        qk_circuit_measure(circuit, 0, 0);
+        qk_circuit_measure(circuit, 1, 1);
+
+        size_t n_inst = qk_circuit_num_instructions(circuit);
+        printf("Original circuit: %zu instructions\n", n_inst);
+
+        const char *tmp_path = "/tmp/jeff_test_output.jeff";
+        if (qiskit_to_jeff(circuit, tmp_path) < 0) {
+            qk_circuit_free(circuit);
+            return 1;
+        }
+        printf("Wrote jeff file, reading back...\n");
+
+        if (jeff_to_qiskit(tmp_path, nullptr) < 0) {
+            qk_circuit_free(circuit);
+            return 1;
+        }
+
+        unlink(tmp_path);
+        qk_circuit_free(circuit);
+        printf("PASS\n");
+        return 0;
+    }


This should not be part of the CLI.

denialhaag · 2026-06-07T22:33:51Z

This does not suffice. Please ensure that the tests define a Qiskit circuit, convert it to a valid jeff module, convert it back to Qiskit, and compare the input and output circuits. The tests should test the conversion of all supported gates and be written as unit tests. Furthermore, please ensure that everything is configured correctly for the tests to run in the CI (e.g., the converter is built, qiskit is defined as a test dependency, etc.).

denialhaag · 2026-06-07T22:38:44Z

+def _build_binary() -> Path:
+    if BINARY.exists():
+        return BINARY
+    result = subprocess.run(
+        ["cmake", "-B", str(BUILD_DIR), str(REPO_ROOT / "tools/qiskit_convert")],
+        capture_output=True,
+        text=True,
+        env={**os.environ, "QISKIT_ROOT": str(QISKIT_SITE)},
+    )
+    if result.returncode != 0:
+        raise RuntimeError(f"cmake configure failed:\n{result.stdout}\n{result.stderr}")
+    result = subprocess.run(
+        ["cmake", "--build", str(BUILD_DIR)],
+        capture_output=True,
+        text=True,
+    )
+    if result.returncode != 0:
+        raise RuntimeError(f"cmake build failed:\n{result.stdout}\n{result.stderr}")
+    return BINARY


This should not be part of the test file.

denialhaag · 2026-06-07T22:43:58Z

+    auto strings = mod.initStrings(4);
+    strings.set(0, "main");
+    strings.set(1, "q");
+    strings.set(2, "c");
+    strings.set(3, "gate");


What are q, c, and gate? Why would they need to be in the list of strings? 🤔

denialhaag · 2026-06-07T22:48:22Z

+    uint32_t n_qubits = qk_circuit_num_qubits(circuit);
+    uint32_t n_clbits = qk_circuit_num_clbits(circuit);
+    size_t n_inst = qk_circuit_num_instructions(circuit);


If I understand this correctly, the conversion treats all circuit qubits as function sources. This should not be happening. All qubits and registers (classical and quantum) should be allocated using the respective jeff operations.

…LI and tests - CMakeLists.txt: Use pre-generated capnp bindings from impl/cpp/src/capnp/ instead of generating at build time - main.cpp: Allocate qubits via jeff alloc ops instead of function sources (remove body.setSources) - main.cpp: Remove useless strings (q, c, gate) — keep only main and custom - main.cpp: Remove 'test' command from CLI — round-trip verification moved to Python test code - tests: Move _build_binary to conftest.py (session-scoped fixture) - tests: Rewrite as proper unit tests with tempfile isolation

…nsive gate tests

feat: rewrite Qiskit converter in C++ with capnp + Qiskit C API

c366625