Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFC] Ansatz Design Doc #5

Merged
merged 8 commits into from
Jan 27, 2020
Merged

Conversation

Cryoris
Copy link
Contributor

@Cryoris Cryoris commented Jan 23, 2020

Migrating from qiskit-community/qiskit-aqua#747.

This design doc/rfc is in jupyter notebook format and schematically follows the md template. As you pointed out in #1, we'd need a ipynb template as well (maybe it's sufficient to just 'translate' the md one?).

What should we do with images contained in the RFC? For now I created a folder according to the RFC naming scheme containing the notebook along with the images.

@claassistantio
Copy link

claassistantio commented Jan 23, 2020

CLA assistant check
All committers have signed the CLA.

@dongreenberg dongreenberg requested review from dongreenberg and removed request for mtreinish January 23, 2020 15:50
@dongreenberg
Copy link
Contributor

Hey Julien, two things:

  1. Newcomers may not know what part of qiskit "Ansatz Design Doc" refers to. Maybe we can change it to "Aqua 0.7 Ansatz Design Doc"?
  2. Could you please copy the following into the top of the doc, under the title, as in the rfc template (and fill it out)? For submitted, you can use the day when you sent it for design review. For deprecated, you can just put "N/A". For status, please put "Accepted."
Status Proposed/Accepted/Deprecated
RFC # ####
Authors First Author (first.author@ibm.com), ...
Deprecates RFC that this RFC deprecates
Submitted YYYY-MM-DD
Updated YYYY-MM-DD

@dongreenberg
Copy link
Contributor

dongreenberg commented Jan 23, 2020

Notes from design review:

Notetaker/Steering: Donny

  • High-level Impressions
    • Kevin (@kdk )
      • Terra can help in some areas, e.g. Barriers for print but not transpilation
      • Should clarify what the Terra objects under the hood will be, or hide them, so as not to confuse users
      • Should think harder about eager transpilation timing, so transpile only happens when we know about the backend
    • Stefan (@stefan-woerner )
      • What is the difference between the three types of terra circuits?
      • Transpilation - there is a difference between optimizing the gate and transpiling for hardware
      • Initialstate, UncertaintyModel - All overlapping, should be made the same
    • Panos (@pbark )
      • Transpilation - Not doable to do pre-transpilation, because it depends on the parameters. Would be hard if doable.
      • Agrees with Stefan that more thought needs to go into organizing Featuremaps, InitialState, etc.
      • Inserting parameters will be difficult and maybe overly complicated
    • Christa (@Zoufalc )
      • We need the gate insertion for gradients
      • Initial state and uncertainty problem isn’t really an ansatz, it’s just a fixed circuit
    • Ali (@ajavadia )
      • Implementation - Should this just be derived from QuantumCircuit, or gate or something else, because it has a lot of overlapping functionality (append, etc.)?
      • Difference between circuit, gate, instr - On our radar. Transpiling only one section of a gate should work by transpiling a circuit, and then calling to_gate(), and using that as is.
    • Steve (@woodsp-ibm )
      • Originally, Aqua came from a place where components were each pieces of circuits. We didn’t care what was under the hood, but want a consistent interface. Whatever we do here should be the same in the all the components.
      • It’s good to have common classes, but not necessary to mush everything, e.g. InitialState, together into one class just to say we did.
      • Parameters, transpile - if it’s not necessary to pass hardware information we shouldn’t. It would be better to just pass the user a circuit and have them transpile.
      • Mutability - If I have an Ansatz and do an append, will the algorithm understand? If you want to build an ansatz, can you just stack some together, instead of building complicated functionality to modify?
    • Donny (@dongreenberg )
      • Like that the purpose of the Anastz as a thing that provides a service of repeating gates, and like Ali’s point that maybe this is just a sparse representation of a circuit.
      • Context on transpile - when we got rid of caching, we started trying to do this eager transpile.
  • Julien (@Cryoris )
    • Philosophy
      • Python is a language with lots of syntactic sugar and just works, and wanted to see that in the ansatz.
      • Wanted things to be easy to understand across Aqua, where if you understand the Ansatz you realize that you understand the Featuremap and other too.
      • Backward compatibility
    • Questions
      • Bounds - The Ansatz has bounds, but doesn’t enforce them. Should a gate have bounds?
        • Unresolved, table for later
      • Can we support a “parameterized_circuit” property in the gate?
        • Kevin: yes
      • Inherit or hold Circuit / Gate / Instruction?
        • Table for later
      • Combining things - InitialState, FeatureMap, etc. motivation is just to simplify these to make them familiar.
  • Ratings
    • All - Conditionally Approve, pending resolving Circuit inheritance vs. encapsulation discussion.

After follow up discussion, it was decided for the Ansatz to hold a gate or circuit, rather than inherit from Gate or QuantumCircuit, to allow for some flexibility as this design settles (inheritance introduces strong coupling which is hard to extricate later), and to allow for diverse Ansatze, such as linear combinations of circuits, not-gate.

Final rating: Approved

Copy link
Contributor

@dongreenberg dongreenberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please just fix changes mentioned in comments above. Also please change the number in the filenames and top section of the doc to 0001.

@dongreenberg dongreenberg changed the title Ansatz Design Doc [RFC] Ansatz Design Doc Jan 24, 2020
@Cryoris
Copy link
Contributor Author

Cryoris commented Jan 24, 2020

@dongreenberg Updated!

@dongreenberg dongreenberg merged commit 515eb55 into Qiskit:master Jan 27, 2020
blakejohnson added a commit that referenced this pull request Oct 27, 2023
* Filled out the Summary, Motivation, and User Benefit sections

* Filled out Alternative Approaches section

* Add Tasks section

* Add to Future Extensions section

* Add more minor edits

* Add Sections (#4)

* add to Future Extensions (re Task in BasePrimitive)

* Array broadcasting figure, example (#5)

* Add section references (#6)

* Add "BindingsArray generalizations" subsection

* Add subsection titles

* Add more BindingsArray examples

* Wording change

* Move the example up (no significant changes)

* Add detail to migration plan (#7)

* Light editing of first few sections.

* Move all of the class ideas into Detailed Design

* Edits in the detailed design subsections

* Edits to migration section

* Flesh out examples more

* update date

* update authors

* switch observable array format

* Small fixes here and there---nothing major.

* Update authors

* Address Chris` "Task" comments

* Further response to Chris' "Task" comments

* alphebetize and edit authors

* rename ResultBundle -> TaskResult

* Address Jim's comment

* Rename ObservablesTask -> EstimatorTask

* Update 0015-estimator-interface.md

Co-authored-by: Takashi Imamichi <31178928+t-imamichi@users.noreply.github.com>

* Update 0015-estimator-interface.md

Co-authored-by: Luciano Bello <bel@zurich.ibm.com>

* Update 0015-estimator-interface.md

Co-authored-by: Takashi Imamichi <31178928+t-imamichi@users.noreply.github.com>

* Update 0015-estimator-interface.md

Co-authored-by: Luciano Bello <bel@zurich.ibm.com>

* Update 0015-estimator-interface.md

Co-authored-by: Blake Johnson <blakejohnson04@gmail.com>

* fix typos in broadcasting examples

* Address Blake's comment about exponential memory overhead for BaseOperator

* Added Migration Examples section

This is following a request to see some simpler examples of what the new
interface would look like.

* Update 0015-estimator-interface.md

Co-authored-by: Elena Peña Tapia <57907331+ElePT@users.noreply.github.com>

* Update 0015-estimator-interface.md

Co-authored-by: Elena Peña Tapia <57907331+ElePT@users.noreply.github.com>

* Fix shapes

* Update 0015-estimator-interface.md

Co-authored-by: Luciano Bello <bel@zurich.ibm.com>

* Removed 'Inner' from figure

* Update 0015-estimator-interface.md

Co-authored-by: Jim Garrison <jim@garrison.cc>

* Update 0015-estimator-interface.md

Co-authored-by: John Lapeyre <jlapeyre@users.noreply.github.com>

* Update 0015-estimator-interface.md

Co-authored-by: Toshinari Itoko <15028342+itoko@users.noreply.github.com>

* Update 0015-estimator-interface.md

Co-authored-by: Toshinari Itoko <15028342+itoko@users.noreply.github.com>

* Update 0015-estimator-interface.md

Co-authored-by: Toshinari Itoko <15028342+itoko@users.noreply.github.com>

* change migration path to versioning

* clarify output type

---------

Co-authored-by: Samantha Barron <sam@sambarron.me>
Co-authored-by: Takashi Imamichi <31178928+t-imamichi@users.noreply.github.com>
Co-authored-by: Luciano Bello <bel@zurich.ibm.com>
Co-authored-by: Blake Johnson <blakejohnson04@gmail.com>
Co-authored-by: Elena Peña Tapia <57907331+ElePT@users.noreply.github.com>
Co-authored-by: Jim Garrison <jim@garrison.cc>
Co-authored-by: John Lapeyre <jlapeyre@users.noreply.github.com>
Co-authored-by: Toshinari Itoko <15028342+itoko@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants