This repository has been archived by the owner on Aug 21, 2023. It is now read-only.
Update qGAN tutorial #1036
Merged
Update qGAN tutorial #1036
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
|
Since the tutorial now works, it needs to be moved out of the Also the authors, affiliation and embedded title image need to go to match the style of the other notebooks. |
|
The qGAN training requires about an hour and can thus not suffice the 3 min cell limit |
|
So unfortunately not, we are bound by tight time limits due to the corresponding time limits in the CI build process. That being said, I think that this tutorial can be slimmed down by decreasing the number of epochs and also through code changes. Indeed, running profile on this training gives data copy at the time. This is an indication that things are not as optimal as they otherwise could be (see below). It is not clear if this is because of excessive circuit generation in Aqua, or if this is something in the transpiler (where most of the time is spent). The runtime is also not linear in the number of epochs. I am not sure if this is to be expected or not. 3000 epochs requires 3.5x longer runtime than a single one times 3000. Also, a single epoch is again dominated by data copies. I leave it to you @kdk, @mtreinish, and @levbishop to figure out where the core bottleneck is here. Slimmed down profile for single epoch: Profile for all 3000: |
|
Taking a quick look at that profile it looks like the overhead from parameter binding. In the shorter term @hhorii has been looking at aqua-side optimizations to leverage aer's parameterized qobj that can significantly reduce this overhead (in that case it was specifically for UCCSD in chemistry applications) see: qiskit-community/qiskit-aqua#1206 We probably can leverage a similar mechanism for this too. |
|
First of all, you are absolutely right about the number of training epochs. The number is far too large. Would it be sufficient to pretrain some epochs and then use them as a starting point instead of random initialization? |
|
@mtreinish Thanks for looking into this! |
nonhermitian
approved these changes
Oct 7, 2020
manoelmarques
pushed a commit
to manoelmarques/qiskit-machine-learning
that referenced
this pull request
Mar 11, 2021
* Update qgans_for_loading_random_distributions.ipynb * move qGAN tutorial from legacy_tutorials to tutorials * Update qgans_for_loading_random_distributions.ipynb * Remove Torch import * change Kernel * Update qgans_for_loading_random_distributions.ipynb Co-authored-by: Paul Nation <nonhermitian@gmail.com>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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
The aim of this PR is to make sure that the qGAN tutorial on how to load random distributions is up to date with recent version of the Qiskit master branches.
Details and comments
Replace
aqua_globals.randomwithnp.random