-
Notifications
You must be signed in to change notification settings - Fork 65
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 direct Chimera unit-cell embedding composite #294
Conversation
__all__ = ('DirectChimeraTilesEmbeddingComposite', | ||
) | ||
|
||
class DirectChimeraTilesEmbeddingComposite(dimod.ComposedSampler): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be a StructuredSampler?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think so
Before I dive in too deeply, it occurs to me that there is overlap with TilingComposite. Maybe we should consider merging these two together? |
@arcondello, there are similarities but this one is specifically for when users want a one-to-one node-qubit fit (no chains) and control of placement. To bring this functionality into Tiling you'd need to (1) flag for single instance, which is simple, (2) enable/disable use of chains, which is a bigger change, (3) add vertical search for Pegasus columns -- I did not do that because I'm not sure there are use cases to justify the extra complications. |
Tiling does not use chains, the "embeddings" are all length-1 chains and just used for convenient mapping. Agree (3) complicates things, but we'll need to handle that case when we adapt TilingComposite to Pegasus anyway... |
I've updated the ReverseAnnealing and Anneal Schedule JNs to use #431-updated TilingComposite instead of the DirectEmbedding class. Closing. |
For several Leap JNs I wrote a function in the past that finds the first available Chimera unit cells to "directly" embed (map each node of a Chimera-indexed graph to one qubit) small problems. The typical use case is placing the problem below without worrying about the working graph of a particular QPU:
I'm now updating the Leap JNs for Pegasus, and went overboard on generalizing that code. I've often found this functionality useful and for Pegasus, with its more complicated coordinates, it's especially helpful, so I thought we might want it in Ocean.
An alternative approach would be to check that all nodes/couplers are in the needed number of unit cells. That code would be simpler but more wasteful. For the target graphs that's not really a big deal, though it's uglier. My main motivations for taking this approach was that it seemed a better approach pedantically for when I thought it would stay in the JNs and it was raining that weekend anyway.
If we want to add this to Ocean, I'd still need to update the RST files.