tqec
tqec copied to clipboard
QCHackers
Framework integration
My surprise was actually about cirq, not qiskit, since I thought cirq was required for stim. It's possible this tool simply gets installed when you installed stim.
On Tue, Oct 7, 2025, 6:10 AM J @.***> wrote:
@.**** commented on this pull request.
In src/tqec/interop/nisq/load.py https://github.com/tqec/tqec/pull/720#discussion_r2410583448:
- qc.h(0)
- qc.cx(0, 3)
- qc.cx(0, 4)
- qc.cx(0, 5)
- qc.cx(0, 6)
- qc.h(0)
- qc.h(1)
- qc.cx(1, 3)
- qc.cx(1, 4)
- qc.cx(1, 7)
- qc.cx(1, 8)
- qc.h(1)
- qc.h(2)
- qc.cx(2, 3)
- qc.cx(2, 5)
- qc.cx(2, 7)
- qc.cx(2, 9)
- qc.h(2)
Yes, we should add it as .qpy, steane.qpy should do it. Do you want to do it or should I do it? I'm fine either way.
In a conversation with @afowler https://github.com/afowler we were surprised that qiskit is not a default dependency already. It used to be, no? Since it's the major such framework, it makes sense to ship it default. At the very least, it keeps our own versioning in check.
As the notebook runs qiskit by default, this suffices to avoid runtime errors.
Other frameworks... Let's pin that until after the next push in this draft PR. I have a suggestion, but best not to mix topics.
— Reply to this email directly, view it on GitHub https://github.com/tqec/tqec/pull/720#discussion_r2410583448, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKAXTF5EAU2X5G5Y2HJHK33WO3TTAVCNFSM6AAAAACIISKLYCVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTGMJQGA2DQMJUHE . You are receiving this because you were mentioned.Message ID: @.***>
cirq, not qiskit
My bad, I mixed them up.
Cirq is not installing at all, even after my last push containing additional dependencies.
I was going to add it explicitly but:
- we don't yet have a cirq steane (in that conversation I mixed up acronyms, we have qiskit and qrisp)
- there's a versioning problem with cirq that would require loosening TQEC's main Python requirements (to include 3.10) and I can't immediately tell if this could have collateral effects.
So, the latest push has a few things worth noting explicitly.
Dependencies
I added a new dependency group to pyproject.toml.
- A foundational install wouldn't install topologiq, nor qiskit, nor qrisp, not any other framework we might add in the future.
- If you add the
docsdependencies, it would install all pre-existing doc dependecies plus topologiq and qiskit because that's all the notebook needs to run in its default configuration - There is an additional
integrationgroup with topologiq and all supported frameworks, which is what you would install if running comprehensive integration tests locally (or, more practically, you woulduv add <applicable_framework>only the framework you're interested in).
The notebook contains instructions to clarify all this. It's easier to explain for an user than to give the full summary above, as they just need to know they need to run the group or add their chosen framework library.
Notebook & circuits Additionally:
- Uploaded the .qpy Yilun linked above to
./assets - Updated the nisq importer and notebook to reflect this
- Updated notebook to consider all feedback in this thread
- Updated notebook to consider above-noted changes to dependencies.
My surprise was actually about cirq, not qiskit, since I thought cirq was required for stim.
@afowler There is now a separate package called stimcirq. We only install Stim without the additional package.
@jbolns Now, the docs build is failing because your notebook needs to be added to docs/gallery/index.rst.
With the exception of the comment I noted explicitly not understanding, the latest push should address all comments thus far.
Changelog summary
The most significant change is that, having externalised the circuits to a corresponding file in ./assets/, it is now possible to import them directly into the notebook.
As a consequence, I went ahead and completely eliminated the ./src/tqec/interops/nisq I had introduced earlier, as it was no longer being used by the notebook.
A few additional notes While I was very sold on the idea of a single notebook for many frameworks, I tried adding a cirq version using the circuit Angela uploaded to the algorithm's folder. At that point, the notebook did become a bit too long. I think two frameworks fit perfectly into a single notebook, but if we are going to add examples of more frameworks, we might want to consider several framework-specific notebooks. Problem is, this would significantly add simulations to the deployment workflow, one full set of sims per notebook.
I think it is okay for a user to focus on a single framework and do a single set of sims per framework. It is not obvious that two different frameworks even with in principle the same quantum circuit will lead to exactly the same lattice surgery and therefore exactly the same simulations.
On Wed, Oct 8, 2025, 3:43 AM J @.***> wrote:
jbolns left a comment (tqec/tqec#720) https://github.com/tqec/tqec/pull/720#issuecomment-3380918000
With the exception of the comment I noted explicitly not understanding, the latest push should address all comments thus far.
Changelog summary The most significant change is that, having externalised the circuits to a corresponding file in ./assets/, it is now possible to import them directly into the notebook.
As a consequence, I went ahead and completely eliminated the ./src/tqec/interops/nisq I had introduced earlier, as it was no longer being used by the notebook.
A few additional notes While I was very sold on the idea of a single notebook for many frameworks, I tried adding a cirq version using the circuit Angela uploaded to the algorithm's folder. At that point, the notebook did become a bit too long. I think two frameworks fit perfectly into a single notebook, but if we are going to add examples of more frameworks, we might want to consider several framework-specific notebooks. Problem is, this would significantly add simulations to the deployment workflow, one full set of sims per notebook.
— Reply to this email directly, view it on GitHub https://github.com/tqec/tqec/pull/720#issuecomment-3380918000, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKAXTBJUH267A4KNNF3WHT3WTTGDAVCNFSM6AAAAACIISKLYCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTGOBQHEYTQMBQGA . You are receiving this because you were mentioned.Message ID: @.***>
Latest push splits the notebook into separate Qiskit and Qrisp notebooks.
Additional notebooks possible by repeating the general recipe in these two notebooks.
- The top of a new notebook would be 100% specific to framework of choice
- The QASM-->PyZX bit might differ for some frameworks, given parsing difficulties
- The PyZX optimisation might vary a little according to the previous step
- The topologiq and TQEC bits are probably almost a copy+paste irrespective of base framework
Hi J,
I am getting the following error at the point of the qiskit_integration.ipynb notebook where we use get_stim_circuit to get the Stim circuit for different bases.
---------------------------------------------------------------------------
AttributeError Traceback (most recent call last)
Cell In[39], line 39
37 # Call Stim circuit generation for all relevant bases
38 if filled_block_graphs:
---> 39 get_stim_circuit(filled_block_graphs)
41 else:
42 print("ERROR. Block didn't run because there is no block_graph or it was not possible to create filled block_graphs from it.")
Cell In[39], line 22, in get_stim_circuit(pre_filled_block_graphs)
20 if block_graph_for_computation:
21 compiled_graph = compile_block_graph(block_graph_for_computation)
---> 22 stim_circuit = compiled_graph.generate_stim_circuit(
23 k=1, noise_model=NoiseModel.uniform_depolarizing(p=0.001)
24 )
25 print("\nFirst 10 lines of Stim circuit (Basis X):\n")
26 print(stim_circuit[:10])
File ~/Projects/dev/ytqec/tqec_pr720/src/tqec/compile/graph.py:501, in TopologicalComputationGraph.generate_stim_circuit(self, k, noise_model, manhattan_radius, detector_database, database_path, do_not_use_database, only_use_database)
465 def generate_stim_circuit(
466 self,
467 k: int,
(...) 473 only_use_database: bool = False,
474 ) -> stim.Circuit:
475 """Generate the ``stim.Circuit`` from the compiled graph.
476
477 Args:
(...) 499
500 """
--> 501 circuit = self.to_layer_tree().generate_circuit(
502 k,
503 manhattan_radius=manhattan_radius,
504 detector_database=detector_database,
505 database_path=database_path,
506 do_not_use_database=do_not_use_database,
507 only_use_database=only_use_database,
508 )
509 # If provided, apply the noise model.
510 if noise_model is not None:
File ~/Projects/dev/ytqec/tqec_pr720/src/tqec/compile/tree/tree.py:311, in LayerTree.generate_circuit(self, k, include_qubit_coords, manhattan_radius, detector_database, database_path, do_not_use_database, only_use_database, lookback)
309 if detector_database is None: # Nothing passed in,
310 if database_path.exists(): # look for an existing database at the path.
--> 311 detector_database = DetectorDatabase.from_file(database_path)
312 else: # if there is no existing database, create one.
313 detector_database = DetectorDatabase()
File ~/Projects/dev/ytqec/tqec_pr720/src/tqec/compile/detectors/database.py:507, in DetectorDatabase.from_file(filepath, format)
501 if format not in DetectorDatabase._READERS:
502 raise TQECError(
503 f"Could not read the database: the provided format {format} is not supported. "
504 + "Supported formats are:\n -"
505 + "\n -".join(DetectorDatabase._READERS.keys())
506 )
--> 507 return DetectorDatabase._READERS[format](filepath)
File ~/Projects/dev/ytqec/tqec_pr720/src/tqec/compile/detectors/database.py:222, in _DetectorDatabaseIO.from_pickle_file(filepath)
220 try:
221 with open(filepath, "rb") as f:
--> 222 database = pickle.load(f)
223 except pickle.PickleError as e:
224 moving_location = filepath.parent / f"faulty_database_{hash(e)}.pkl"
AttributeError: Can't get attribute 'ExtendedBasis' on <module 'tqec.utils.enums' from '/Users/kabirdubey/Projects/dev/ytqec/tqec_pr720/src/tqec/utils/enums.py'>
I think you need to add these lines to src/tqec/utils/enums,py, as in main:
class PauliBasis(Enum):
X = "X"
Y = "Y"
Z = "Z"
def __str__(self) -> str:
return self.value # pragma: no cover
def to_extended_basis(self) -> ExtendedBasis:
"""Return ``self`` as an extended basis."""
return ExtendedBasis(self.value)
class ExtendedBasis(Enum):
X = PauliBasis.X.value
Y = PauliBasis.Y.value
Z = PauliBasis.Z.value
H = "H"
def __str__(self) -> str:
return self.value # pragma: no cover
Also, this is probably because this PR is still in progress, but I wasn't able to use the docs/_examples_database pre-computed in the branch to generate the plots. I needed to delete it and then wait it out. I'll make a discussion post trying to find standard operating proceduring for using the database in demos.
Thanks, Kabir
Latest push has some minor updates AND updates branch against main.
@KabirDubey Want to re-pull the branch and give it a go again to see if it was something related to applying recent updates on main to this branch?
I'm not sure that'll do the trick, for reasons below. But worth trying
Could this be a Mac thing?
- The contents of
src/tqec/utils/enums.pyin the latest version ofmainI've pulled (just now) are different than what you suggest. In particular, there is noPauliBasisorExtendedBasiswhatsover. - The database note is odd. I'm using it as given, and it's been working from the first try in both Linux and Windows machines.
- Can you run other gallery notebooks like, for instance, the manually-built Steane?
from the first try
- from the first successful try, after I figured out Linux versioning problems (but that wasn't related to the notebooks or the database, it was a flawed numpy re-versioning)
@jbolns - Quick heads up: found and fixed a coordinate transformation bug in topologiq.py while testing the integration.
https://github.com/tqec/tqec/pull/726
Merged a fork-to-branch PR just now. #726, by @SMC17.
It's failed some automatic checks, but:
- the import function is a significant improvement to existing one.
- it's adds a comprehensive set of tests.
Will pull and see about fixing whatever caused the failed checks.
I have to say I do not currently understand why the type checks are failing.
There were two types of error.
- Something related to variable naming. That is no longer appearing.
- Something related to how I call topologiq's main runner.
It shouldn't be failing.
- It's unlikely related to the merge by @SMC17 because they failing files were not touched by that merge.
- The runner function hasn't changed from the last working version.
- I did remove one line, but then I introduced the line back in (with a different
fig_datapayload but nonetheless the same kind of object as was being passed in last working version). - It works on my computer. =D
- The GH Pages deployment succeeds, which I think means it does build.
Will sleep on it, but if anyone's got a clue what is going on, by all means.
@purva-thakre @Zhaoyilunnn ?
@Azuya334 , as you have a physics background, would you happen to know how to design a circuit in any of the following?
- https://quantumai.google/qsim
- https://github.com/ProjectQ-Framework/ProjectQ
- https://github.com/CQCL/hugr
- https://github.com/CQCL/guppylang
- https://github.com/CQCL/selene
The main ask underneath this PR is to create a Steane encoding using one framework, export it to QASM, and see if it loads to PyZX (qsim might load directly to PyZX without a need for QASM, though).
These frameworks above have not yet been taken.
Example implementations in docs/gallery/qiskit_integration.ipynb and docs/gallery/qrisp_integration.ipynb, with the corresponding circuits assets/steane_qiskit.qpy and assets/steane_qrisp.py.
@jbolns I learned to use qsim a while ago for a class. I can go pick it up. I am not too familiar with implementing Steane code, I will go read the examples.
I have to say I do not currently understand why the type checks are failing.
There were two types of error.
- Something related to variable naming. That is no longer appearing.
- Something related to how I call topologiq's main runner.
It shouldn't be failing.
- It's unlikely related to the merge by @SMC17 because they failing files were not touched by that merge.
- The runner function hasn't changed from the last working version.
- I did remove one line, but then I introduced the line back in (with a different
fig_datapayload but nonetheless the same kind of object as was being passed in last working version).- It works on my computer. =D
- The GH Pages deployment succeeds, which I think means it does build.
Will sleep on it, but if anyone's got a clue what is going on, by all means.
@purva-thakre @Zhaoyilunnn ?
It also works on my computer (linux ubuntu 22.04) with the same python version, ty version, and other package versions as the github ci env. I think this may be a bug of ty? I also don't understand why.
Since the error occurs in the notebook of docs directory, I would suggest to skip ty checking on these notebooks. Because, in any way, they are not the source of tqec framework itself. I don't know if others agree on this point.
https://github.com/tqec/tqec/pull/728
This topologiq PR (tqec/topologiq#17) directly supports the Qiskit/Qrisp integration work:
Pipeline Strengthening: The Qiskit/Qrisp integration relies on the PyZX → topologiq → TQEC pipeline. This work strengthens the topologiq component:
- Performance: 1.78x faster processing reduces pipeline latency
- Reliability: Comprehensive testing validates topologiq behavior
- Debugging: Visualization tools help debug graph conversions
- Reproducibility: Forced parameters enable pipeline debugging
Testing Patterns: The testing patterns in this PR can inform integration testing:
- Parametrized tests for variant validation
- Reproducible test execution with forced parameters
- Statistical validation of performance improvements
- Zero-warning test output standards
Integration Debugging: If the Qiskit/Qrisp demos encounter topologiq issues:
# Reproduce specific pipeline behavior
result = graph_manager_bfs(
circuit,
force_src_id=specific_id,
force_src_kind=specific_kind,
...
)
# Visualize topologiq's graph processing
visualize_exploration_3d(explored, path, src, tgt)
Happy to help debug any topologiq-related integration issues!
I took weekend of, @SMC17, so I didn't answer your comment from yesterday asking what to prioritise. Apologies for that.
Very excited to start looking at topologiq's 17 this coming week. It looks very cool!
For clarity in expectations. I was able to merge #726 to this PR without extensive review because it was a merge-to-branch. We will still review all code in this PR extensively prior merging to main. Topologiq's 17 is a merge-to-main, so I will need to go over everything manually and with a lot of patience.
@inmzhang Is the new folder structure in last push along the lines of what you'd suggested.
Also, I still don't get why uv run ty check is failing here. When I test locally, the checks clear. @purva-thakre ?
@jbolns Sorry for the late reply. I was traveling and ended up sick as well. I will go through the comments in this PR today.
@jbolns The docs build is failing due to the messages provided here: https://github.com/tqec/tqec/actions/runs/18498220732/job/52707752197?pr=720#step:7:102
@inmzhang Is the new folder structure in last push along the lines of what you'd suggested. Also, I still don't get why
uv run ty checkis failing here. When I test locally, the checks clear. @purva-thakre ?
@jbolns You might want to check the version of ty you have locally. I have it setup such that the latest version of uv is installed in the CI. What is installed by the CI for this PR is screenshotted below:
Looks like a new version tag was added 5 days ago. https://github.com/astral-sh/ty/tags
I think it is okay for a user to focus on a single framework and do a single set of sims per framework. It is not obvious that two different frameworks even with in principle the same quantum circuit will lead to exactly the same lattice surgery and therefore exactly the same simulations.
I agree with @afowler comment mostly from the POV of how long it will take for the docs build to run if we demo the Steane code circuit for all frameworks.
What are everyone's thoughts on having one notebook only for framework integration, where the code randomly selects which framework's circuit to use, prints a couple of lines about this, and implements the integration pipeline?
The ty issue is resolved. I reckon it deserves a explicit summary. Resolved by including all optional parameters in function call. It was failing, seemingly, because some optional parameters were deemed necessary, which then messed up the position where it expected other parameters.
Summary of last push.
- Improves dependency management
- Improves readability of notebooks
- Eliminates ty failures
- Realises the multi-gallery approach: standard gallery + integration gallery.
Pending Per my count, the pending issues to discuss are:
- Optimising tests introduced in #726 by using parametrize to condense multiple unit tests like this into a single unit test
- Reducing docstrings of
.../interops/pyzx/topologiq.pybug-fix introduced in #726 to the standard TQEC length and style - Deciding between ONE or MANY notebooks approach, alongside their placement.
One or many?
I think it is okay for a user to focus on a single framework and do a single set of sims per framework. It is not obvious that two different frameworks even with in principle the same quantum circuit will lead to exactly the same lattice surgery and therefore exactly the same simulations.
I think @afowler meant it is preferable to use several notebooks focusing each on a single framework. This avoids a Qiskit user having to worry about Qrisp/pytket/Cirq code and vice-versa.
One notebook only for framework integration, where the code randomly selects which framework's circuit to use
This is similar to my initial idea of letting users select a framework, with the improvement that this would automatically run different frameworks every time there's a merge. So, I can't say I fundamentally disagree. It would be fine.
I'm siding with different notebook for each framework tested successfully because they're easier to read by users.
Having said that, I would prefer if the designers of the circuits for this or that framework prepare and push their respective notebooks. Not too worried about the Qiskit and Qrisp notebooks since both @purva-thakre and @Zhaoyilunnn are established contributors. But newer contributors who go over the pain of designing a circuit and testing it should get the lines/credit.
Having said that, I would prefer if the designers of the circuits for this or that framework prepare and push their respective notebooks.
If you have access to their notebooks in the shared folder, you can add them as co-authors in github. https://gist.github.com/lisawolderiksen/f9747a3ae1e58e9daa7d176ab98f1bad
reach out to each person for their github handle.
This is similar to my initial idea of letting users select a framework, with the improvement that this would automatically run different frameworks every time there's a merge. So, I can't say I fundamentally disagree. It would be fine.
I'm siding with different notebook for each framework tested successfully because they're easier to read by users.
My primary motivation for this is that the docs build time is going to increase per framework notebook. Locally, if I build the docs for the first time (without the cache), each Steane circuit simulation is going to take at least 1 hour per notebook (the Steane blockgraph notebook takes half this time on my thinkpad). If I look at the frameworks considered in Austin's presentation, this PR will add at least 6 hours for the first local build (if the Steane simulation per framework takes just an hour).
The time in the CI is lower due to caching, for now, because the cache exists[^1]. But for the cache to exist locally, you have to at least build it once i.e., wait 6+ hours the first time. Plus, you would be against using make clean locally because the docs build time has increased so much.
Edit: I just pulled the notebooks in this PR locally. Each notebook takes approximately 25-30 minutes on my Thinkpad with make clean html and 15 minutes with make html. The latter is using the locally saved cache. Multiply these times by 6-7 for each framework. Assume double the time for the CI (without a proper cache).
[^1]: Plus, something is wrong with the CI cache. I won't be able to look it over until after next week.