Open
Conversation
julescraquelin
requested review from
eflurin,
nbourdau,
rlescanne and
sjezouin
|
Questions from a beginner in Pull Request :
|
nbourdau
requested changes
Nov 19, 2020
nbourdau
left a comment
nbourdau
left a comment
There was a problem hiding this comment.
Seb said:
* there are many commits that I added on master a while ago. So should they be removed from this PR ?
Yes, they should, they must be removed. The problem is that the content of external several branches rebased into the Autotune_Jules branch while they should not have.
I have not yet dived into the bulk the Autotune code but before starting we must have clean state:
- The autotune commit must start off from the QuanticParis master branch since it seems to be the main development branch. If it requires stuff from Manu's branches, they must be considered to be reviewed and merged into master.
- The history of development of Autotune does not matter as it is a single piece of code not requiring changes in pyEPR. Hence all commit regarding autotune should be squashed altogether.
I will go depeer in review in the commit days, but anyways, the comments I've just mentioned are orthogonal to the future comments I could makes so you can already start address them.
|
Also I would like to propose: what about doing some of the review of QuanticParis's pyEPR on http://gerrithub.io/ ? |
Cherry-picked from Manu's work
julescraquelin
force-pushed
the
Autotune_Jules
branch
from
df6a7f4 to
b948495
Compare
Automatization of the tuning of a chip. Recover and sort the HFSS data processed by pyEPR : frequencies, Quality factors, Kers and Chis. Defines a cost function with the user's criterion and the previous HFSS data. Fulfill the user's criterion by testing several values of HFSS parameters until finding the zero of the cost function. Current method is 'Bayesian Optimization' but please add other methods.
julescraquelin
force-pushed
the
Autotune_Jules
branch
from
b948495 to
c426d64
Compare
|
In the end can we accept this PR ? |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Added 2 files to autotune a quantic circuit :