Log time for execute steps #3922
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
javabster
requested review from
1ucian0,
ajavadia,
chriseclectic,
diego-plan9,
kdk,
lcapelluto,
mtreinish and
taalexander
as code owners
kdk
reviewed
Mar 30, 2020
There was a problem hiding this comment.
Thanks @javabster , sorry for the slow review. This looks good so far. Few comments below.
time.time returns a time in seconds. Co-Authored-By: Kevin Krsulich <kevin@krsulich.net>
Change LOG to logger for consistency Co-Authored-By: Kevin Krsulich <kevin@krsulich.net>
log_output better than output Co-Authored-By: Kevin Krsulich <kevin@krsulich.net>
This reverts commit 86219c7.
kdk
previously approved these changes
Apr 6, 2020
mtreinish
previously approved these changes
Apr 6, 2020
There was a problem hiding this comment.
This LGTM, thanks for doing this. It's good to get more log messages being emitted to help in debugging. My only nit would be the unit choice of milliseconds especially for transpile() since I'm used to working on the order of seconds or minutes for the things I'm normally doing. That being said I have a couple of small suggestions in the tests, but nothing worth blocking over and something easy enough to fix in a follow up PR.
| @@ -696,6 +699,37 @@ def test_measure_doesnt_unroll_ms(self, optimization_level): | |||
| self.assertEqual(qc, out) | |||
|
|
|||
|
|
|||
| class StreamHandlerRaiseException(StreamHandler): | |||
There was a problem hiding this comment.
While this class is only 3 lines I feel like we should move it some where common to avoid having a copy in every place we're asserting log output.
Co-Authored-By: Matthew Treinish <mtreinish@kortar.org>
Co-Authored-By: Matthew Treinish <mtreinish@kortar.org>
Co-Authored-By: Matthew Treinish <mtreinish@kortar.org>
mtreinish
approved these changes
Apr 6, 2020
kdk
added
the
Changelog: New Feature
faisaldebouni
pushed a commit
to faisaldebouni/qiskit-terra
that referenced
this pull request
Aug 5, 2020
* Added logging for total transpile time * Added logging for total assembly time * Added total schedule time logging * Added total schedule time logging * Added logging for job submission and fixed linting errors * Fixed linting errors * Update qiskit/compiler/assemble.py time.time returns a time in seconds. Co-Authored-By: Kevin Krsulich <kevin@krsulich.net> * Update qiskit/execute.py Change LOG to logger for consistency Co-Authored-By: Kevin Krsulich <kevin@krsulich.net> * Update test/python/basicaer/test_qasm_simulator.py log_output better than output Co-Authored-By: Kevin Krsulich <kevin@krsulich.net> * Made updates to job submission based on PR comments * Fixed styling error * Fixed test error * Revert "Update qiskit/compiler/assemble.py " This reverts commit 86219c7. * Update test/python/compiler/test_transpiler.py Co-Authored-By: Matthew Treinish <mtreinish@kortar.org> * Update test/python/compiler/test_assembler.py Co-Authored-By: Matthew Treinish <mtreinish@kortar.org> * Update test/python/basicaer/test_qasm_simulator.py Co-Authored-By: Matthew Treinish <mtreinish@kortar.org> Co-authored-by: Kevin Krsulich <kevin@krsulich.net> Co-authored-by: Kevin Krsulich <kevin.krsulich@ibm.com> Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR fixes issue #3805
I've added logging for Total Transpile Time, Total Schedule Time, Total Assembly Time and backend job submission. Am happy to add more if there are further suggestions. Have added tests where possible, although sometimes was unsure of which file was the best place to put the tests (I'm still relatively new to Qiskit contributions).
Details and comments