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

Pengyu/wrapper #178

Open
wants to merge 36 commits into
base: master
Choose a base branch
from
Open

Pengyu/wrapper #178

wants to merge 36 commits into from

Conversation

liupengy19
Copy link
Collaborator

Add rpc wrapper for quartz

Copy link
Collaborator

@xumingkuan xumingkuan left a comment

Choose a reason for hiding this comment

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

Great work!

Comment on lines +6 to +7
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -O3")
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -O3")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please remove these two lines because we want to set -O0 on debug mode (see lines 82-95).

Suggested change
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -O3")
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -O3")

If you need to set -O3 also in CMAKE_C_FLAGS, you can modify these lines.

#include <assert.h>

namespace quartz {
class B2Gate : public Gate {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
class B2Gate : public Gate {
/**
* A barrier to block 2 qubits and prevent optimizations across it.
*/
class B2Gate : public Gate {

Comment on lines +1473 to +1475
// while (p >= 2 * PI) {
// p -= 2 * PI;
// }
Copy link
Collaborator

Choose a reason for hiding this comment

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

I just merged #177 into the master branch and please rebase this PR onto it. I think it's better to split out the logic changes to existing code if possible.

#include <vector>
using namespace quartz;

class SuperContext {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add some comment -- what is a supercontext?

#include "oracle.h"

using namespace quartz;
std::shared_ptr<SuperContext> get_context_(const std::string gate_set,
Copy link
Collaborator

Choose a reason for hiding this comment

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

If it returns a SuperContext, it's better for the function name to include "super".

std::string new_circ =
optimize_(my_circ, cost, timeout_type, timeout_value, supercontext);
std::cout << new_circ << std::endl;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would put all test files in the src/test folder. But feel free to leave it as is for now if the wrapper is an individual component.

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.

2 participants