-
Notifications
You must be signed in to change notification settings - Fork 62
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
Improvements to final logical-physical
qubit mapping, documentation
#1562
Comments
Tagging possible relevant people following PR #1500: |
Opened a PR with very small changes, that make it for me. |
@GuillermoAbadLopez thanks for your interest and the issue! In a sense, the current transpiler is more or less in an intermediate stage. It is usable, but not incredibly optimized and deeply reviewed. That's just for you to gauge the depth you want to reach in there. Moreover, Qibo is suffering from a common Python shortcoming, which is exposing essentially everything from a package, and relying on conventions for internals recognition (which are clear up to a point, and need to be supplemented with project-wide policies - ttbomu). In this context, we (developers) are currently intending the transpiler implementation as mostly internal, while the user-facing portion should be just limited to the transpiler setup and configuration. custom_passes = [Preprocessing(), Random(), ShortestPaths(), Unroller(native_gates=natives)]
# Create a transpiler pipeline with hardware configuration
custom_pipeline = Passes(custom_passes, connectivity=star_connectivity()) https://qibo.science/qibo/stable/code-examples/advancedexamples.html#how-to-transpile-a-circuit Every other function or method (i.e. other than In any case, now, the preferred way to rearrange the mapping is actually the qibo/src/qibo/models/circuit.py Line 1096 in d32efb8
The current steps should be certainly reviewed, together with the way the information is passed around. The docstrings should be generally improved, and synced with the current flow. The mechanisms for retrieving permutations (only relevant for certain steps) should be standardized (for which I'd recommend just to drop them, and rely on the Any help to improve the transpiler is vastly welcome and encouraged :) |
Hey @alecandido, thank you for such a detailed answer!
I figured when I saw that the module is not in the docs API reference, we were aware don't worry 😅. But after checking the PRs and the last version of the code, it seems to me it was usable from our side 👌
I wondered if I was being too annoying complaining about something I saw peeking at the code itself, sorry about that :)
That is exactly how we are doing things around here, so everything is perfect until here 👍 Origin of doubts:
Actually, my initial doubts (about keys/values in ![]() I'll try:
Nice, thank you for the info! |
Not at all. I'm truly glad that you showed interest, but just sorry about the limitations. Thanks for your in-depth investigation! In case there are things about which you're unsure (something that doesn't look clearly as an issue, or a proposal that is vague enough to call for feedback to iterate), consider to post it as a discussion https://github.com/qiboteam/qibo/discussions. If instead, at any time, you'll be looking for an actual chat, consider using our Matrix channel |
Cool, noted it down, thanks for everything 👍 |
Closed by #1563 |
Update:@alecandido after trying both with the returned (Instead Also, I'm a bit worried about what you commented on the return type of Passes changing in each child, since seems you want to change it (which makes sense ofc). But since right now it's the most useful to us, we will use it, and whenever you change it, we will think of a better solution 👍. Possible solution:Maybe a possible solution would be, to add an extra wire name attribute to the FarewellI didn't want to open a new discussion/Issue, since from our side it seems to be working just fine now (using the |
@GuillermoAbadLopez no worries, it's not going to happen anytime soon, and if you wish you can be part of the process (or even lead it). For as long as it is not changed, you'll be free to use it at will. Moreover, while Qibo itself is now pretty lenient on semantic versioning (by contrast, Qibolab is now much stricter - and sooner or later even Qibo will follow suit), we're still versioning.
The problem I see is that you are following the path of "qubit tracking" during the circuit operation. So, to me, the In any case, I never went too deep into transpilation, so I'm certainly up for discussion, and open to insightful inputs! |
@alecandido nice to hear, and I will gladly take part in it!
Right
Right, it is truly not trivial, but if you specifically are adding swaps to the original circuit of the user, you can maybe argue, that those are remapping the circuit while the others don't. Even more if you do it automatically without the user knowing for example. Which can be our case, since we want to automatically (But ofc, we will always inform of the used qubits, right now I'm implementing to |
Not expecting anything beyond that. I don't know to what extent this transpilation support is among your company's priorities, but the message was that in case you'll be free to contribute to Qibo as well for that purpose (and we'll be glad to accept it).
True, but that is potentially unstable if you then attempt optimizing your gates, since the swap you introduced may be further decomposed and then partly optimized away. |
🙌 🙌
Right, but any further optimization should have the same global unitary, for the same order of qubits concretely, no? So only changing the |
@alecandido if you are curious about how we are going to use it, you can take a look in: (Take into account, that the PR is still not ready for merge though) |
Correct. In my former comment, I focused on the unitary input, and forgot that you also need to permute its output (either manually, which is the simplest, or by further So, while the input permutation is encoded in the |
@alecandido right, In the end, it was handy to have both the
But of course, maybe you could encode the Hopefully, this feedback is useful, thanks for building such a cool library 👍 |
Yeah, indeed. Either that, or we should always return the But here I will ask for @stavros11's opinion as well :)
You're more than welcome ^^ |
Hello, I'm a developer for Qilimanjaro, and we are currently implementing your placing/routing into our stack,
here I leave some small improvements that would have helped me, hopefully, they are useful to you.
qibo 0.2.15
Small problems:
All the docstrings for the call of the
Router
/Placer
passes, say something similar to:"final physical-logical qubits mapping."
for explaining the final mapping/layout, without explicitly stating what the keys and values are. And for an external user, although I can guess the format is:keys-values
, I think stating it explicitly would be better.Assuming the order of:
physical-logical
in the dosctring corresponds tokeys-values
, then there might be a contradiction. In theSabre
router call it sayslogical-physical
, but then the function it ends up calling, has the oppositephysical-logical
:Possible solutions:
Router
/Placer
passes.The text was updated successfully, but these errors were encountered: