Skip to content
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.

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

Add Rust PropertySet #12209

Open
mtreinish opened this issue Apr 18, 2024 · 4 comments
Open

Add Rust PropertySet #12209

mtreinish opened this issue Apr 18, 2024 · 4 comments
Labels
mod: transpiler Issues and PRs related to Transpiler Rust This PR or issue is related to Rust code in the repository type: feature request New feature or request

Comments

@mtreinish
Copy link
Member

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.

@mtreinish mtreinish added type: feature request New feature or request Rust This PR or issue is related to Rust code in the repository mod: transpiler Issues and PRs related to Transpiler labels Apr 18, 2024
@mtreinish mtreinish added this to the 1.2.0 milestone Apr 18, 2024
@jakelishman
Copy link
Member

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
Copy link
Member Author

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
Copy link
Member

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
Copy link
Member

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.

@mtreinish mtreinish modified the milestones: 1.2.0, 1.3 beta Jul 25, 2024
@mtreinish mtreinish modified the milestones: 1.3 beta, 1.3.0 Aug 8, 2024
@raynelfss raynelfss removed this from the 1.3.0 milestone Oct 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mod: transpiler Issues and PRs related to Transpiler Rust This PR or issue is related to Rust code in the repository type: feature request New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants