Skip to content
This repository has been archived by the owner on Aug 19, 2023. It is now read-only.

Assemble circuits and transpiler peakmem benchmarks #641

Conversation

kdk
Copy link
Member

@kdk kdk commented Oct 21, 2019

Adds a peakmem tracking for transpiler_level benchmarks, and a benchmark for assemble_circuits.

@kdk kdk force-pushed the assemble_circuits-and-transpiler-peakmem-benchmarks branch from 4a70a89 to 8e65e26 Compare October 24, 2019 15:13
@kdk kdk force-pushed the assemble_circuits-and-transpiler-peakmem-benchmarks branch from 8e65e26 to b14d240 Compare October 24, 2019 15:34
Copy link
Member

@mtreinish mtreinish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, a couple inline questions but nothing blocking

test/benchmarks/assembler.py Outdated Show resolved Hide resolved


class AssemblerBenchmarks:
params = ([1, 2, 5, 8],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to expand the number of qubits here at all. Just to get a feel for how this scales as we approach the largest sizes?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes and no. There are performance issues in assemble for larger n_qubits (e.g. we do some List.indexs that should be dictionary look ups) but right now, far and away, performance scales with the number of gates. I limited n_qubits in the interest of keeping the benchmark runtime reasonable, but we can always expand it.

(As an aside, since we generate random circuits by depth and not by gate count, increasing n_qubits does dramatically increase the benchmark runtime but only because the test circuits contain more gates, so there's not a lot of interesting information there, at least right now. Maybe we should update random_circuit to accept either depth or gate_count.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good point, I figured it was for run time. Yeah most of the time is spent looping over the instructions in experiments.

If we wanted to add a gate_count parameter it'd be simple to do (my first implementation of random circuit which we didn't use in terra actually did it this way). We should do this before we adopt any benchmarks with it though because we'd have to regenerate any data.

mtreinish and others added 2 commits October 24, 2019 14:31
@mtreinish mtreinish merged commit 41f3f6d into Qiskit:master Oct 25, 2019
@mtreinish
Copy link
Member

Ci Passed, not that we actually run benchmarks in CI (still waiting on the new asv release with the --strict flag). Travis issue seemed to report the job incorrectly so I went ahead and merged this.

jakelishman pushed a commit to jakelishman/qiskit-terra that referenced this pull request Aug 1, 2023
…tapackage#641)

Adds a peakmem tracking for transpiler_level benchmarks, and a
benchmark for assemble_circuits.

* Add peakmem versions of transpiler level benchmarks.

* Add assemble_circuit benchmark.

* Import assemble from qiskit.compile.
jakelishman pushed a commit to jakelishman/qiskit-terra that referenced this pull request Aug 11, 2023
…tapackage#641)

Adds a peakmem tracking for transpiler_level benchmarks, and a
benchmark for assemble_circuits.

* Add peakmem versions of transpiler level benchmarks.

* Add assemble_circuit benchmark.

* Import assemble from qiskit.compile.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants