Conversation
|
One or more of the following people are relevant to this code:
|
Pull Request Test Coverage Report for Build 21163412204Details
💛 - Coveralls |
gadial
left a comment
There was a problem hiding this comment.
LGTM. My only concern is that I'm not sure how deterministic is the transpilation stage (not across different runs but across future qiskit versions) but since this was already done for the QASM benchmark it makes sense to follow suit.
Thanks Gadi. This is a good point, and in fact I just commented on something very related in a different PR: see this comment, where the quality of the output circuit depends on the transpiler seed. My take on this is that we should be aware of this randomness and investigate outliers to see if they are not hiding a real bug. |
Summary
Some of our existing ASV benchmarks were broken. This commit fixes these (though alternatively I could have simply deleted some of the benchmarks as well).
Details and Comments
For
TestCircuitManipulate: #13919 deleted the huge qasm fileqv_N100_12345.qasmand modified thecircuit_constructionbenchmark to construct the relevant quantum circuit instead of loading it from qasm. This PR applies the same fix to themanipulatebenchmark as well.For
PassBenchmarks: theApplyLayoutpass is a transformation pass, and in particular changes the DAG from virtual to physical (which also changes the register names). If we want to save the DAG before theApplyLayoutpass (so that we can run theApplyLayoutto measure its performance), we shoulddeepcopy.For
RoutedPassBenchmarks:self.routed_dagwas not defined. Note thatGateDirection/CheckGateDirectionraise an error if the DAG's connectivity does not adhere to the coupling map. At some point in the past, the test includedStochasticSwapto route the circuit (and defineself.routed_dagin the first place), however the line got removed with the removal of stochastic swap. As we still need to route the circuit for this benchmark, I have reincluded the removed line withSabreSwapinstead.