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.

Sign up for GitHub

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

Jump to bottom

add circuit optimization framework #83

Merged
merged 3 commits into from
Mar 26, 2019
Merged

add circuit optimization framework #83

merged 3 commits into from
Mar 26, 2019

Conversation

hhorii
Copy link
Collaborator

@hhorii hhorii commented Mar 12, 2019

Summary

This PR provides circuit optimization for QasmController.

Details and comments

Current design is here.

  1. Add optimization in constructor.
QasmController::QasmController() {
  add_optimization(ReduceNop());
}
  1. Apply optimization in optimize_circuit().
template <class State_t>
void QasmController::optimize_circuit(const Circuit &input_circ,
                                      Circuit &output_circ) const {

  Circuit working_circ = input_circ;
  for (std::shared_ptr<CircuitOptimization> opt: optimizations)
    opt->optimize_circuit(working_circ);

  output_circ = working_circ;
}

@CLAassistant
Copy link

CLAassistant commented Mar 12, 2019 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@hhorii hhorii changed the title add circuit optimization framework [WIP] add circuit optimization framework Mar 12, 2019
@hhorii hhorii changed the title [WIP] add circuit optimization framework add circuit optimization framework Mar 17, 2019
chriseclectic
chriseclectic reviewed Mar 19, 2019
View reviewed changes
Copy link
Member

@chriseclectic chriseclectic left a comment

Choose a reason for hiding this comment

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

Overall this looks very good! I think the functions add_circuit_optimization optimize_circuit and member optimizations_ should be moved to the Base::Controller class so they can be inherited by all controller subclasses.

Then the QasmController will just have the specific optimization (eg ReduceNop) added in its initialization (which could also be added to the other controllers).

src/framework/circuitopt.hpp Outdated

void CircuitOptimization::set_config(const json_t& config) {
for ( auto it = config.begin(); it != config.end(); ++it )
config_[it.key()] = it.value();
Copy link
Member

Choose a reason for hiding this comment

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

I think this loop can just be replaced with config_ = config;.

src/simulators/qasm/basic_optimization.hpp Outdated

std::vector<Operations::Op>::iterator it = circ.ops.begin();
while (it != circ.ops.end()) {
std::cerr << it->name << std::endl;
Copy link
Member

Choose a reason for hiding this comment

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

Can you change this to std::clog << it->name << std::endl; ?

src/simulators/qasm/qasm_controller.hpp Outdated
@@ -73,6 +79,14 @@ class QasmController : public Base::Controller {
// Clear the current config
void virtual clear_config() override;

// Add circuit optimization
template <typename Type>
inline auto add_optimization(Type&& opt)
Copy link
Member

Choose a reason for hiding this comment

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

This should be added to the Base::Controller class so it can be used by all the controller subclasses. I would also rename it to be add_circuit_optimization to be specific about what optimizations we are talking about

src/simulators/qasm/qasm_controller.hpp Outdated
@@ -186,13 +200,22 @@ class QasmController : public Base::Controller {
// Initial statevector for Statevector simulation method
cvector_t initial_statevector_;

std::vector<std::shared_ptr<CircuitOptimization>> optimizations;
Copy link
Member

Choose a reason for hiding this comment

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

optimizations_; with underscore for private/protected class member

Also could you add a short documentation comment above for what this is.

src/simulators/qasm/qasm_controller.hpp Outdated
@@ -299,8 +322,12 @@ void QasmController::initialize_state(const Circuit &circ,
template <class State_t>
void QasmController::optimize_circuit(const Circuit &input_circ,
Copy link
Member

Choose a reason for hiding this comment

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

This function should also be moved to the Base::Controller class

src/simulators/qasm/qasm_controller.hpp Outdated
@@ -299,8 +322,12 @@ void QasmController::initialize_state(const Circuit &circ,
template <class State_t>
void QasmController::optimize_circuit(const Circuit &input_circ,
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should also change the function definition to be:

Circuit optimize_circuit(const Circuit &circ) const

So that it returns the output circuit object and is used like

auto opt_circ = optimize_circuit<State_t>(circ);

rather than

Circuit opt_circ;
optimize_circuit<State_t>(circ, opt_circ);

@chriseclectic
Copy link
Member

@hhorii If you make the changes above (and update the conflicting files) we can get this merged

@chriseclectic chriseclectic merged commit a298c42 into Qiskit:master Mar 26, 2019
dcmckayibm pushed a commit to dcmckayibm/qiskit-aer that referenced this pull request Nov 3, 2019
@hhorii @chriseclectic
add circuit optimization framework (Qiskit#83)
029df7e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chriseclectic chriseclectic chriseclectic approved these changes

@atilag atilag Awaiting requested review from atilag

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@hhorii @CLAassistant @chriseclectic