Wrap qiskit.Aer and qiskit.IBMQ with lazy loading object #5619

mtreinish · 2021-01-12T23:48:28Z

Summary

This commit migrates the qiskit.Aer and qiskit.IBMQ module attributes to
be lazy loading instances of lazy loading wrapper classes. The intent
here is to avoid importing from qiskit-aer or qiskit-ibmq-provider from
qiskit-terra, while this is safe while the packages share a shared
namespace we've been actively working to remove the use of namespace
packaging (see #5089, #4767, and #559) and the circular
dependency caused by re-exporting these attributes is blocking progress
on this. By using a lazy loading wrapper class we avoid an import type
circular dependency and opportunistically use qiskit-aer and/or
qiskit-ibmq-provider at runtime only if they're present after everything
is imported. This also may have some benefit for the overall import
performance of qiskit (being tracked in #5100) as it removes qiskit-aer
and qiskit-ibmq-provider from the import path unless they're being used.

Details and comments

Fixes #5532

This commit migrates the qiskit.Aer and qiskit.IBMQ module attributes to be lazy loading instances of lazy loading wrapper classes. The intent here is to avoid importing from qiskit-aer or qiskit-ibmq-provider from qiskit-terra, while this is safe while the packages share a shared namespace we've been actively working to remove the use of namespace packaging (see Qiskit#5089, Qiskit#4767, and Qiskit#559) and the circular dependency caused by re-exporting these attributes is blocking progress on this. By using a lazy loading wrapper class we avoid an import type circular dependency and opportunistically use qiskit-aer and/or qiskit-ibmq-provider at runtime only if they're present after everything is imported. This also may have some benefit for the overall import performance of qiskit (being tracked in Qiskit#5100) as it removes qiskit-aer and qiskit-ibmq-provider from the import path unless they're being used. Although the presence of qiskit.__qiskit_version__ might prevent any performance improvements as it still imports all the elements to get version information (and will be tackled in a separate PR). Fixes Qiskit#5532

nonhermitian · 2021-01-13T11:28:42Z

I thought these preloading of modules was to be removed. Are these to be removed later?

mtreinish · 2021-01-13T12:06:10Z

I thought these preloading of modules was to be removed. Are these to be removed later?

My thinking was that this is a prerequisite for a deprecation and eventual removal of the attributes (it will probably be a long deprecation because these are widely used today). This gives us a place to raise a deprecation warnings (which wouldn't be an option otherwise while we still support py3.6) in the future while also fixing a potential import cycle today so we can move forward on removing the namespace packages..

nonhermitian · 2021-01-13T12:17:11Z

Great. Tired of getting aer warnings when Aer doesn't even build on the platform.

… lazy-load-aer

mtreinish · 2021-01-13T19:23:37Z

Ugh, this is failing the tutorials because ignis is using from qiskit import Aer to determine if Aer is available: https://github.com/Qiskit/qiskit-ignis/blob/master/qiskit/ignis/verification/topological_codes/fitters.py#L29-L34 which I wasn't able to preserve with this approach. To unblock I'll have to update the ignis code and switch CI to use ignis master (and we'll have to co-release ignis and terra)

mtreinish · 2021-01-13T19:47:06Z

Actually now I'm not sure what's going on. That line is wrong (per the upgrade notes) but it's not clear why it's using BasicAer (which is what I'm assuming is the cause of the timeouts) there. Aer is installed in the tutorial job and from qiskit import Aer will always work with this PR so it should be using Aer. The issue we had in the tests with that for a skip condition was that CI jobs without aer installed would fail because the skip condition was based on from qiskit import Aer raising an importerror which it doesn't anymore

In Qiskit/qiskit#5619 how the qiskit.Aer provider gets loaded will be changing and it will always be present, not just when Aer is installed. In the topological codes fitters it was checking for the presence of Aer using qiskit.Aer which won't continue to work moving forward. To avoid this potential issue (and potentially unblock Qiskit/qiskit#5619) this commit switches to using 'from qiskit.providers.aer import Aer' instead which lives in the qiskit-aer project itself and will always import error if aer isn't installed.

This reverts commit ac9611c.

kdk

Looks good, one comment on the release note.

releasenotes/notes/lazy-load-provider-factory-bdfb3925a2514ef7.yaml

In Qiskit#5582 we added a script that is used to ensure we don't import slow optional dependencies in the default 'import qiskit' path. Since that PR merged we've managed to remove a few more packages from the default import path (see Qiskit#5619, Qiskit#5485, and Qiskit#5620) which we should add the same checks to to ensure we don't regress and accidently start importing them again in the default path. This commit adds all these packages to the list of imports not allowed as part of 'import qiskit'.

In #5582 we added a script that is used to ensure we don't import slow optional dependencies in the default 'import qiskit' path. Since that PR merged we've managed to remove a few more packages from the default import path (see #5619, #5485, and #5620) which we should add the same checks to to ensure we don't regress and accidently start importing them again in the default path. This commit adds all these packages to the list of imports not allowed as part of 'import qiskit'.

In Qiskit/qiskit#5619 how the qiskit.Aer provider gets loaded will be changing and it will always be present, not just when Aer is installed. In the topological codes fitters it was checking for the presence of Aer using qiskit.Aer which won't continue to work moving forward. To avoid this potential issue (and potentially unblock Qiskit/qiskit#5619) this commit switches to using 'from qiskit.providers.aer import Aer' instead which lives in the qiskit-aer project itself and will always import error if aer isn't installed. (cherry picked from commit 2e3298f)

* Wrap qiskit.Aer and qiskit.IBMQ with lazy loading object This commit migrates the qiskit.Aer and qiskit.IBMQ module attributes to be lazy loading instances of lazy loading wrapper classes. The intent here is to avoid importing from qiskit-aer or qiskit-ibmq-provider from qiskit-terra, while this is safe while the packages share a shared namespace we've been actively working to remove the use of namespace packaging (see Qiskit#5089, Qiskit#4767, and Qiskit#559) and the circular dependency caused by re-exporting these attributes is blocking progress on this. By using a lazy loading wrapper class we avoid an import type circular dependency and opportunistically use qiskit-aer and/or qiskit-ibmq-provider at runtime only if they're present after everything is imported. This also may have some benefit for the overall import performance of qiskit (being tracked in Qiskit#5100) as it removes qiskit-aer and qiskit-ibmq-provider from the import path unless they're being used. Although the presence of qiskit.__qiskit_version__ might prevent any performance improvements as it still imports all the elements to get version information (and will be tackled in a separate PR). Fixes Qiskit#5532 * Fix lint * Fix test lint * DNM: test with ignis patch * Revert "DNM: test with ignis patch" This reverts commit ac9611c. * Use ignis from source for tutorial job * Update release note to be more clear

…it#5619) * Wrap qiskit.Aer and qiskit.IBMQ with lazy loading object This commit migrates the qiskit.Aer and qiskit.IBMQ module attributes to be lazy loading instances of lazy loading wrapper classes. The intent here is to avoid importing from qiskit-aer or qiskit-ibmq-provider from qiskit-terra, while this is safe while the packages share a shared namespace we've been actively working to remove the use of namespace packaging (see Qiskit/qiskit#5089, Qiskit/qiskit#4767, and Qiskit/qiskit#559) and the circular dependency caused by re-exporting these attributes is blocking progress on this. By using a lazy loading wrapper class we avoid an import type circular dependency and opportunistically use qiskit-aer and/or qiskit-ibmq-provider at runtime only if they're present after everything is imported. This also may have some benefit for the overall import performance of qiskit (being tracked in Qiskit/qiskit#5100) as it removes qiskit-aer and qiskit-ibmq-provider from the import path unless they're being used. Although the presence of qiskit.__qiskit_version__ might prevent any performance improvements as it still imports all the elements to get version information (and will be tackled in a separate PR). Fixes Qiskit/qiskit#5532 * Fix lint * Fix test lint * DNM: test with ignis patch * Revert "DNM: test with ignis patch" This reverts commit ac9611c3ace30d101a70bda06e1987df14662182. * Use ignis from source for tutorial job * Update release note to be more clear

mtreinish requested review from manoelmarques, woodsp-ibm and a team as code owners January 12, 2021 23:48

mtreinish added Changelog: API Change Include in the "Changed" section of the changelog Changelog: Removal Include in the Removed section of the changelog labels Jan 12, 2021

mtreinish force-pushed the lazy-load-aer branch from 42a48fa to a77faa3 Compare January 13, 2021 00:01

mtreinish added this to the 0.17 milestone Jan 13, 2021

mtreinish mentioned this pull request Jan 13, 2021

Add namespace redirect hook for qiskit-aer #5089

Merged

mtreinish added 4 commits January 13, 2021 07:52

Fix lint

c12c4c0

Merge branch 'master' into lazy-load-aer

1ed63e8

Fix test lint

ea8cc7d

Merge branch 'lazy-load-aer' of github.com:mtreinish/qiskit-core into…

ce34092

… lazy-load-aer

mtreinish mentioned this pull request Jan 13, 2021

Update aer check in topological codes qiskit-community/qiskit-ignis#546

Merged

DNM: test with ignis patch

ac9611c

mtreinish added 2 commits January 13, 2021 17:06

Revert "DNM: test with ignis patch"

6192981

This reverts commit ac9611c.

Use ignis from source for tutorial job

689f28d

mtreinish added the performance label Jan 14, 2021

mtreinish added 5 commits January 27, 2021 15:20

Merge remote-tracking branch 'origin/master' into lazy-load-aer

224c659

Merge branch 'master' into lazy-load-aer

8600253

Merge branch 'master' into lazy-load-aer

78f9217

Merge branch 'master' into lazy-load-aer

59905ec

Merge branch 'master' into lazy-load-aer

e25fde8

mtreinish mentioned this pull request Feb 2, 2021

Imports: Speed improvement for importing subpackages #5100

Closed

mtreinish added 2 commits February 3, 2021 16:47

Merge branch 'master' into lazy-load-aer

2d21def

Merge branch 'master' into lazy-load-aer

20ccd56

kdk previously approved these changes Feb 4, 2021

View reviewed changes

releasenotes/notes/lazy-load-provider-factory-bdfb3925a2514ef7.yaml Show resolved Hide resolved

Update release note to be more clear

4aac883

mtreinish dismissed kdk’s stale review via 4aac883 February 5, 2021 16:44

Merge branch 'master' into lazy-load-aer

c82f230

kdk approved these changes Feb 5, 2021

View reviewed changes

kdk added the automerge label Feb 5, 2021

mergify bot merged commit 9d1d6a1 into Qiskit:master Feb 5, 2021

mtreinish mentioned this pull request Feb 5, 2021

Update slow optional import list to include additional packages #5798

Merged

mtreinish mentioned this pull request Mar 15, 2021

Create function to set user config #5943

Merged

jakelishman mentioned this pull request Nov 25, 2021

Define __all__ for qiskit #7274

Merged

mtreinish deleted the lazy-load-aer branch November 25, 2021 12:49

Guillermo-Mijares-Vilarino mentioned this pull request Apr 27, 2023

Migrate qiskit-ibmq-provider Qiskit/qiskit-metapackage#1718

Merged

jakelishman mentioned this pull request Aug 7, 2023

Set up infrastructure for qiskit-tutorials migration #10443

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Wrap qiskit.Aer and qiskit.IBMQ with lazy loading object #5619

Wrap qiskit.Aer and qiskit.IBMQ with lazy loading object #5619

mtreinish commented Jan 12, 2021 •

edited

Loading

nonhermitian commented Jan 13, 2021

mtreinish commented Jan 13, 2021

nonhermitian commented Jan 13, 2021

mtreinish commented Jan 13, 2021

mtreinish commented Jan 13, 2021

kdk left a comment

Wrap qiskit.Aer and qiskit.IBMQ with lazy loading object #5619

Wrap qiskit.Aer and qiskit.IBMQ with lazy loading object #5619

Conversation

mtreinish commented Jan 12, 2021 • edited Loading

Summary

Details and comments

nonhermitian commented Jan 13, 2021

mtreinish commented Jan 13, 2021

nonhermitian commented Jan 13, 2021

mtreinish commented Jan 13, 2021

mtreinish commented Jan 13, 2021

kdk left a comment

Choose a reason for hiding this comment

mtreinish commented Jan 12, 2021 •

edited

Loading