Move heavy visualization imports outside base namespace imports #4958
Conversation
This file contains hidden or 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
Importing IPython takes at least >150ms and is only used directly in one function (and then only if it's in jupyter). This is a pretty heavy import to run unconditionally at the module level, because we end up wasting time if IPython is installed even if we're not inside jupyter confirming the module is present despite never being able to use it. This commit moves it to runtime to avoid inside the function to avoid this import penalty, it will add som small overhead to the function but since its not performance critical this is a fine tradeoff.
This commit moves the import of the qiskit visualization module for the pass manager drawer from the module level in qiskit.transpiler.passmanager to a run time import in the draw() method. The visualization module import time is fairly heavy (when matplotlib is installed) because of its usage of matplotlib. By having the import at the module level this forces this import time penaly on just a bare import of qiskit even if no visualization is going to be used. This is because the passmaanger gets pulled in from the top level namespaces of qiskit. To avoid this unecessary overhead this makes it a runtime import so unless you go to use the passmanager.draw() method it won't be imported. This will add some overhead to the draw() method (especially if matplotlib hasn't already been imported) but since visualization methods aren't performance critical and are typically quite slow this isn't a a problem.
|
To see the impact of this here is an After applying this PR and removing the visualization imports it becomes: |
kdk
reviewed
Aug 20, 2020
There was a problem hiding this comment.
👍 on the import time improvements.
This takes qiskit.visualization out of the default set of modules that are loaded by import qiskit (and so takes it out of tab completion). Is it possible to keep the import time improvement but still load the non-heavy parts of qiskit.visualization?
It should be possible, but this basically means we have to move any matplotlib import to be at runtime instead of at the module level (and then add visualization back to the top level). The only complexity is there are a couple of places we import |
Moving the visualization import to runtime has a backwards compatibility issue in that the visualization module is no longer imported as part of qiskit. This can potentially break users since visualization functionality would be present without a second import. This reverts this change to avoid that breaking change. A follow-on PR will try to address the import time performance from matplotlib in a different way. This reverts commit bcdb37f.
This reverts commit 2967f0b.
|
I looked into making matplotlib imports happen at runtime and there is a different backwards compatibility concern around the |
kdk
approved these changes
Aug 28, 2020
mergify bot
added a commit
that referenced
this pull request
Aug 28, 2020
* Make ipython detection occur at runtime Importing IPython takes at least >150ms and is only used directly in one function (and then only if it's in jupyter). This is a pretty heavy import to run unconditionally at the module level, because we end up wasting time if IPython is installed even if we're not inside jupyter confirming the module is present despite never being able to use it. This commit moves it to runtime to avoid inside the function to avoid this import penalty, it will add som small overhead to the function but since its not performance critical this is a fine tradeoff. * Move PassManager.draw() visualization import to runtime This commit moves the import of the qiskit visualization module for the pass manager drawer from the module level in qiskit.transpiler.passmanager to a run time import in the draw() method. The visualization module import time is fairly heavy (when matplotlib is installed) because of its usage of matplotlib. By having the import at the module level this forces this import time penaly on just a bare import of qiskit even if no visualization is going to be used. This is because the passmaanger gets pulled in from the top level namespaces of qiskit. To avoid this unecessary overhead this makes it a runtime import so unless you go to use the passmanager.draw() method it won't be imported. This will add some overhead to the draw() method (especially if matplotlib hasn't already been imported) but since visualization methods aren't performance critical and are typically quite slow this isn't a a problem. * Fix docs build * Revert "Move PassManager.draw() visualization import to runtime" Moving the visualization import to runtime has a backwards compatibility issue in that the visualization module is no longer imported as part of qiskit. This can potentially break users since visualization functionality would be present without a second import. This reverts this change to avoid that breaking change. A follow-on PR will try to address the import time performance from matplotlib in a different way. This reverts commit bcdb37f. * Revert "Fix docs build" This reverts commit 2967f0b. Co-authored-by: Kevin Krsulich <kevin.krsulich@ibm.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> (cherry picked from commit 4cbc751)
mergify bot
added a commit
that referenced
this pull request
Aug 31, 2020
… (#4997) * Make ipython detection occur at runtime Importing IPython takes at least >150ms and is only used directly in one function (and then only if it's in jupyter). This is a pretty heavy import to run unconditionally at the module level, because we end up wasting time if IPython is installed even if we're not inside jupyter confirming the module is present despite never being able to use it. This commit moves it to runtime to avoid inside the function to avoid this import penalty, it will add som small overhead to the function but since its not performance critical this is a fine tradeoff. * Move PassManager.draw() visualization import to runtime This commit moves the import of the qiskit visualization module for the pass manager drawer from the module level in qiskit.transpiler.passmanager to a run time import in the draw() method. The visualization module import time is fairly heavy (when matplotlib is installed) because of its usage of matplotlib. By having the import at the module level this forces this import time penaly on just a bare import of qiskit even if no visualization is going to be used. This is because the passmaanger gets pulled in from the top level namespaces of qiskit. To avoid this unecessary overhead this makes it a runtime import so unless you go to use the passmanager.draw() method it won't be imported. This will add some overhead to the draw() method (especially if matplotlib hasn't already been imported) but since visualization methods aren't performance critical and are typically quite slow this isn't a a problem. * Fix docs build * Revert "Move PassManager.draw() visualization import to runtime" Moving the visualization import to runtime has a backwards compatibility issue in that the visualization module is no longer imported as part of qiskit. This can potentially break users since visualization functionality would be present without a second import. This reverts this change to avoid that breaking change. A follow-on PR will try to address the import time performance from matplotlib in a different way. This reverts commit bcdb37f. * Revert "Fix docs build" This reverts commit 2967f0b. Co-authored-by: Kevin Krsulich <kevin.krsulich@ibm.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> (cherry picked from commit 4cbc751) 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 moves to particularly heavy visualization imports outside of the base
qiskitnamespace import. The first is moving theIPythonimport from the single qubit transition visualization out of the module level and into the function. This is only ever used when running inside jupyter (it's used to embed the animation in html so jupyter can display it) and adds unnecessary overhead to the rest of the visualization subpackage import if ipython is installed but not in a jupyter context. The second change made is to move the visualization import for the pass managerdraw()method to be at run time. Having it at the module level adds a slow matplotlib to the base qiskit namespace because the passmanager gets exposed via the top level api.Details and comments