-
Notifications
You must be signed in to change notification settings - Fork 15
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
New feature: Global phase linear constraints #62
Conversation
…and set target as the phaseless target
Codecov ReportBase: 96.05% // Head: 96.13% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #62 +/- ##
==========================================
+ Coverage 96.05% 96.13% +0.07%
==========================================
Files 13 13
Lines 1876 1939 +63
==========================================
+ Hits 1802 1864 +62
- Misses 74 75 +1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added comments
ref_nonzero_r = 0 | ||
ref_nonzero_c = 0 | ||
if data["are_gates_real"] | ||
for i=1:n_r, j=1:n_c |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lines 728-734 applies only for all-real gates, which is not covered in the test. Could you add a test to cover this within "QC_model Tests: Global phase constraints"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added unit test to cover all-real case as well
return ref_nonzero_r, ref_nonzero_c | ||
end | ||
|
||
function _get_ref_nonzero_index_of_original_target(T::Array{Complex{Float64},2}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function isn't covered in the tests. Do we need this separately from the previous function _get_ref_nonzero_index_of_real_target
. If possible, I would combine them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a unit test for this as well.
I see where you are coming from; I could add an option to make it one function but I feel it is probably easier to understand to have 2 separate functions for 2 things that I consider very different objects. Like apples and pears.
- One is just a complex array 'target' as it is an input
- One is a QuantumCircuitModel.data.target_gate thing which is under the hood a real array but nonetheless is this part of this magical type QuantumCircuitModel
let me know what you think.
docs/src/quickguide.md
Outdated
@@ -46,7 +46,7 @@ QCOpt takes two types of user-defined input specifications. The first type of in | |||
| `elementary_gates` | Vector of all one and two qubit elementary gates. The menagerie of quantum gates currently supported in QCOpt can be found in [gates.jl](https://github.com/harshangrjn/QuantumCircuitOpt.jl/blob/master/src/gates.jl). | | |||
| `target_gate` | Target unitary gate which you wish to decompose using the above-mentioned `elementary_gates`.| | |||
| `objective` | Choose one of the following: (a) `minimize_depth`, which minimizes the total number of one- and two-qubit gates. For this option, include `Identity` matrix in the above-mentioned `elementary_gates`, (b) `minimize_cnot`, which minimizes the number of CNOT gates in the decomposition. (default: `minimize_depth`) | | |||
| `decomposition_type` | Choose one of the following: (a) `exact_optimal`, which finds an exact, provably optimal, decomposition if it exists, (c) `exact_feasible`, which finds any feasible exact decomposition, but not necessarily an optimal one if it exsists (c) `approximate`, which finds an approximate decomposition if an exact one does not exist; otherwise it will return an exact decomposition (default: `exact_optimal`)| | |||
| `decomposition_type` | Choose one of the following: (a) `exact_optimal`, which finds an exact, provably optimal, decomposition if it exists, (b) `exact_feasible`, which finds any feasible exact decomposition, but not necessarily an optimal one if it exsist, (c) `exact_optimal_global_phase`, which finds an optimal circuit decomposition if it exists, up to a global phase (d) `approximate`, which finds an approximate decomposition if an exact one does not exist; otherwise it will return an exact decomposition (default: `exact_optimal`)| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please correct exsist
to exists
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
including this in the new commit
src/data.jl
Outdated
ref_nonzero_r, ref_nonzero_c = QCO._get_ref_nonzero_index_of_original_target(params["target_gate"]) | ||
global_phase = angle(params["target_gate"][ref_nonzero_r, ref_nonzero_c]) | ||
is_target_real_up_to_phase = QCO.is_gate_real(exp(-im*global_phase)*params["target_gate"]) | ||
if is_target_real_up_to_phase | ||
return real(exp(-im*global_phase)*params["target_gate"]), !is_target_real |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this also be written as a general evaluation for global_phase = 0
and then evaluate the global phase of target if exact_optimal_global_phase
is true? Also, we should add a test to cover this case where target is real in a global_phase
sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
implemented the global_phase 0, added tests and including this in the new commit
src/log.jl
Outdated
global_phase = 1 | ||
|
||
if data["decomposition_type"] in ["exact_optimal_global_phase"] | ||
n_r = size(target_gate)[1] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could these refs be directly obtained using your helper function in utility.jl?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed so this uses: _get_ref_nonzero_index_of_original_target
src/constraints.jl
Outdated
function constraint_gate_target_condition_compact_global_phase_real(qcm::QuantumCircuitModel) | ||
|
||
max_depth = qcm.data["maximum_depth"] | ||
n_r = size(qcm.data["gates_real"])[1] | ||
n_c = size(qcm.data["gates_real"])[2] | ||
|
||
U_var = qcm.variables[:U_var] | ||
|
||
ref_nonzero_r, ref_nonzero_c = QCO._get_ref_nonzero_index_of_real_target(qcm.data::Dict{String,Any}) | ||
|
||
for i=1:n_r, j=1:n_c | ||
if isapprox(qcm.data["target_gate"][i,j], 0, atol=1E-6) | ||
JuMP.@constraint(qcm.model, U_var[i,j,max_depth] == 0) | ||
else | ||
JuMP.@constraint(qcm.model, U_var[i,j,max_depth]*qcm.data["target_gate"][ref_nonzero_r,ref_nonzero_c] == qcm.data["target_gate"][i,j]*U_var[ref_nonzero_r,ref_nonzero_c,max_depth]) | ||
end | ||
end | ||
return |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function needs to be tested in unit tests. Also, I think this and constraint_gate_target_condition_compact_global_phase_complex
could be combined into a single function?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
combined the two global_phase_constraints
unit test now covering both cases
…l phase constraints
Thanks @szabino-mrszabo |
No description provided.