Repository navigation
Use benchpress for PGO test load - #17441
Matthew Treinish (mtreinish) wants to merge 2 commits into
Conversation
Previously for PGO test load we were running a subset of circuits from benchpress through the transpiler in addition to the unit tests. After recent Qiskit releases we are significantly faster to run the full benchpress benchmark suite. Locally it can take between 15-30min depending on the hardware being run on. This is in the time frame for what we can realistically run during the publish pipeline. This commit updates the PGO script to run the unit tests and the full benchpress benchmark suite. We will need to see how this actually behaves in the CI environment. While it might be fast locally, it might be prohibitively slow in the CI environment. So we'll have to evaluate after testing if this will work. We can reduce this to run a subset of benchpress to provide more coverage than what the current script has, but with more modest runtime if needed.
Matthew Treinish (mtreinish)
added
on hold
|
One or more of the following people are relevant to this code:
|
Jake Lishman (jakelishman)
left a comment
Jake Lishman (jakelishman)
left a comment
There was a problem hiding this comment.
I worry that this will over-couple the test set of Qiskit to one benchmark suite - benchpress has some really pretty opinionated / particular ways it does things, and I'm not sure it represents fully realistic workflows for a range of users.
I don't have much of a better suggestion, though.
| QISKIT_PARALLEL=FALSE stestr run --abbreviate | ||
|
|
||
| python tools/pgo_scripts/test_utility_scale.py | ||
| git clone https://github.com/Qiskit/benchpress --depth=1 |
There was a problem hiding this comment.
This is grim for a build script...
Assuming we can't actually package benchpress in some sane manner, can we consider moving the checkout up into the GitHub Actions logic, feed in a path to benchpress via environment variables for the script (or something), and pin the commit of benchpress in the GitHub Action, for auditability in the future with the build?
(I know our builds aren't perfectly reproducible yet anyway, but it does feel awkward having a completely unpinned build dependency.)
There was a problem hiding this comment.
We can't do that because of the container isolation on Linux. We need to run it as part of the cibuildwheel pieces so that it gets into the container for the build. I was doing those mostly to see what it looks like in practice. For reproducibility we should definitely pin the checkout to a a specific sha1 which should be sufficient. We can pass that in from a gha env var if you prefer having it there.
There was a problem hiding this comment.
That's fair. Functionally I don't mind where the pin is, provided it's easy to update.
I agree with all of this, but it's what we have now. I would rather we had something that we had in the qiskit repo and covered the scale and scope that benchpress does. But we don't so this is the best source of test load we can use for right now. I'm not tied to it though so we can revisit this when/if we have something better. |
|
Well, the x86_64 job didn't finish running benchpress within 6 hours and the job timed out. It's definitely not fast enough to do a full benchpress run. We can look at a subset to see if that is fast enough. |
Previously for PGO test load we were running a subset of circuits from benchpress through the transpiler in addition to the unit tests. After recent Qiskit releases we are significantly faster to run the full benchpress benchmark suite. Locally it can take between 15-30min depending on the hardware being run on. This is in the time frame for what we can realistically run during the publish pipeline. This commit updates the PGO script to run the unit tests and the full benchpress benchmark suite.
We will need to see how this actually behaves in the CI environment. While it might be fast locally, it might be prohibitively slow in the CI environment. So we'll have to evaluate after testing if this will work. We can reduce this to run a subset of benchpress to provide more coverage than what the current script has, but with more modest runtime if needed.
AI/LLM disclosure