[core] Add metadata map for decomposition patterns #3587
Conversation
lmondada
force-pushed
the
lm/pattern-metadata
branch
from
623f8ae to
f7bd459
Compare
| } | ||
|
|
||
| // Test 3: Verify pattern decompositions produce only target gates | ||
| TEST_F(DecompositionPatternsTest, DecompositionProducesOnlyTargetGates) { |
There was a problem hiding this comment.
This test is quite verbose/ugly (see also createTestModule). Is there a better way to write it?
Command Bot: Processing... |
|
CUDA Quantum Docs Bot: A preview of the documentation can be found here. |
lmondada
force-pushed
the
lm/pattern-metadata
branch
2 times, most recently
from
66dcd3e to
0727390
Compare
Signed-off-by: Luca Mondada <[email protected]>
Signed-off-by: Luca Mondada <[email protected]>
lmondada
force-pushed
the
lm/pattern-metadata
branch
from
0727390 to
12252ad
Compare
|
|
||
| // Test 1: Verify the total number of registered decomposition patterns | ||
| TEST_F(DecompositionPatternsTest, TotalPatternCount) { | ||
| auto patternNames = cudaq::DecompositionPatternType::RegistryType::entries(); |
There was a problem hiding this comment.
Wondering if we can fetch the entries just once and use it in all tests, as we are not changing anything in it?
There was a problem hiding this comment.
Yes we can, but in this case I don't think it's useful: entries() returns an iterator, which from the implementation seems very cheap (pretty much free) to construct.
| auto [gatePrefix, gateNum] = splitGateAndControls(gate); | ||
|
|
||
| for (auto targetGate : targetGates) { | ||
| auto [tPrefix, tNum] = splitGateAndControls(targetGate); |
There was a problem hiding this comment.
Just a thought. What if we already store targetGates in a map with prefix as key and num as value? That way we don't have to iterate the list.
There was a problem hiding this comment.
👍 have implemented your suggestion.
sacpis
left a comment
sacpis
left a comment
There was a problem hiding this comment.
Overall this PR looks good to me. Would like @schweitzpgi to have a look at it as well.
|
Seems like few tests are failing in CI. |
|
Adding @khalatepradnya to take a look at it as well. |
Indeed! My guess is that this is due to a change in ordering of the patterns in the |
2 participants
Description
This PR creates a system to register Decomposition patterns and associate metadata to them. This will be useful in an upcoming PR, where the metadata about what gate type is matched and what gate type is introduced by a rewrite will be used to select the subsets of pattern rewrites useful to target a specified target basis (see #3581).
With or without macro?
I tried to avoid using a macro here, but the same ~15 lines are repeated 31 times (once for each decomposition pattern).
A macro can remove this redundancy. What do you prefer?
To make reviewing easier, the first commit introduces all the changes without using a macro. The second commit then introduces the macro and removes redundancies. Getting rid of the macro is thus as simple as removing the second commit, if that's desired.