Extend ASV tests with QASM3 dump #13334
Conversation
There was a problem hiding this comment.
Fwiw, I don't think we #13327 fixed the perf regression that triggered me to open that PR, but it's still worthwhile.
Could you add an additional tests: one that defines a couple of entirely custom gates (at least one parametric and one non-parametric) and uses the same gate several times in the input circuit?
There's more other cases that I'm considering as well, but they're sufficiently esoteric that they're probably not worth encoding into ASV benchmarks (the speed of the dump after a round-trip through OQ2/OQ3/QPY, for example, especially for those that cause the loader to output non-standard gates). Actually, maybe the dump after a QPY round-trip is reasonable, since that's a situation that's actually a lot more likely to come up.
Pull Request Test Coverage Report for Build 11954744748Details
💛 - Coveralls |
Done in 1443081 |
|
One or more of the following people are relevant to this code:
|
There was a problem hiding this comment.
This LGTM, this will be good coverage to get. I left an inline comment about a potential change in the benchmark version for the basis_gate update. Other than that do you have example output on how long these take now. So we get a feeling for how much time we're adding per commit to a benchmark run?
| @@ -35,7 +35,7 @@ def setup(self, n_qubits, depth): | |||
| n_qubits, depth, measure=True, conditional=True, reset=True, seed=seed, max_operands=2 | |||
| ) | |||
| self.fresh_dag = circuit_to_dag(self.circuit) | |||
| self.basis_gates = ["u1", "u2", "u3", "cx", "iid"] | |||
| self.basis_gates = ["u1", "u2", "u3", "cx", "id"] | |||
There was a problem hiding this comment.
This is clearly correct, but I think this might invalidate the results because it changes the hash of the code and asv will treat this as a new version. So I think we should fix it, but lets wait to merge this until after 1.3.0rc1 is tagged so we don't interfere with the collected data
Summary
Inspired by the performance regression fixed in #13327 , a benchmark test for QASM3 dumper.