Qiskit / qiskit

Qiskit is an open-source SDK for working with quantum computers at the level of extended quantum circuits, operators, and primitives.
https://www.ibm.com/quantum/qiskit
Apache License 2.0
4.84k stars 2.29k forks source link

Add Rust `PropertySet` #12209

Open mtreinish opened 2 months ago

mtreinish commented 2 months ago

What should we add?

Right now the property set exists solely in Python space as a dict subclass that defaults to None on missing elements:

https://github.com/Qiskit/qiskit/blob/stable/1.0/qiskit/passmanager/compilation_status.py#L20-L24

As part of the work in #12208 we will need a rust structure that enables us to share data between passes like the property set does in Python space. The ideal case would be if we just replaced the property set with a similar construct that was built in rust, but this might be difficult because a python dict is flexible with regards to typing and rust would enable us to do that by itself. One option might be to have a enum value type where one variant is a PyObject, but then we have variants for all of the types we share between passes in rust space. Another alternative is that we define a custom struct that does a mapping with strict typing in rust space for known names (like layout, etc) and defaults to a pyobject for other field names.

jakelishman commented 2 months ago

Imo the two fields with hard-defined types are layout and final_permutation/final_layout, and we should just promote those to be direct attributes on the Rust DAGCircuit object as they form a core part of the true IR. The rest of the PropertySet can be free form pyobjects, imo.

mtreinish commented 2 months ago

I agree on that point, the ones I was more concerned about were passes like Collect2qBlocks and ConsolidateBlocks. But I guess we can just make custom pyclasses for the cases where we share data between passes like that so there is a no-conversion path and just add a dict lookup to when we call the rust component to pull the data. I'll leave this issue open though until we have a concrete pattern established.

jakelishman commented 2 months ago

Oh yeah, I'm in favour of that too, assuming we're sticking with the idea of a PropertySet at all tbh - I could easily be convinced that a free-form "data dump" isn't the cleanest design, since we don't actually do much hot-swapping out of inner passes.

Within PropertySet, I think having everything be erased to PyAny, and passes that explicitly share data attempt to downcast back to the correct Rust types is probably the best way to keep the spirit of PropertySet through Rust space, while allowing it to be fully inspectable after-the-fact from Python space.

jakelishman commented 3 weeks ago

For inherent parts of the IR (like the layout things I mentioned above), one thing we can do to support the move from the PropertySet to the transpiler IR is to have all Qiskit passes just look for those properties on the DAGCircuit, and the PassManager execution framework can be updated to sync the two fields after each pass runs.