-
Notifications
You must be signed in to change notification settings - Fork 26
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
Delayed Phase Correction #397
base: main
Are you sure you want to change the base?
Changes from 1 commit
c81eb40
5eb3747
8c9d539
1cd55fd
256f588
d218f42
af8ee8f
eb35989
b2e3a92
848e9d7
7690afa
d334f58
267c2b1
f58a738
3b3f028
e6c3348
f45ac80
cad2a04
c02c8ed
0608ae9
e8d4794
b8423cc
995ed9d
6a3bafa
dac77bb
60f64ca
4291548
245f55c
a0652f0
aaeb22f
4b48b48
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -44,7 +44,7 @@ void CliffordSynthesizer::synthesize(const Configuration& config) { | |
encoderConfig.solverParameters = configuration.solverParameters; | ||
encoderConfig.useMultiGateEncoding = | ||
requiresMultiGateEncoding(encoderConfig.targetMetric); | ||
encoderConfig.gateSet = configuration.gateSet; | ||
encoderConfig.gateSet = configuration.gateSet; | ||
encoderConfig.ignoreRChanges = config.delayPaulis; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not name these two options the same? |
||
|
||
if (configuration.heuristic) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,10 +8,10 @@ | |
#include "LogicUtil/util_logicblock.hpp" | ||
#include "cliffordsynthesis/Tableau.hpp" | ||
#include "cliffordsynthesis/encoding/MultiGateEncoder.hpp" | ||
#include "cliffordsynthesis/encoding/PhaseCorrectionEncoder.hpp" | ||
#include "cliffordsynthesis/encoding/SingleGateEncoder.hpp" | ||
#include "operations/OpType.hpp" | ||
#include "utils/logging.hpp" | ||
#include "cliffordsynthesis/encoding/PhaseCorrectionEncoder.hpp" | ||
|
||
#include <chrono> | ||
#include <cstddef> | ||
|
@@ -56,21 +56,24 @@ void SATEncoder::createFormulation() { | |
initializeSolver(); | ||
|
||
S = config.targetTableau->hasDestabilizers() && | ||
config.initialTableau->hasDestabilizers() | ||
? 2U * N | ||
: N; | ||
config.initialTableau->hasDestabilizers() | ||
? 2U * N | ||
: N; | ||
|
||
tableauEncoder = std::make_shared<TableauEncoder>(N, S, T, lb, config.ignoreRChanges); | ||
tableauEncoder = | ||
std::make_shared<TableauEncoder>(N, S, T, lb, config.ignoreRChanges); | ||
tableauEncoder->createTableauVariables(); | ||
tableauEncoder->assertTableau(*config.initialTableau, 0U); | ||
tableauEncoder->assertTableau(*config.targetTableau, T); | ||
|
||
if (config.useMultiGateEncoding) { | ||
gateEncoder = std::make_shared<MultiGateEncoder>( | ||
N, S, T, tableauEncoder->getVariables(), lb, config.gateSet, config.ignoreRChanges); | ||
N, S, T, tableauEncoder->getVariables(), lb, config.gateSet, | ||
config.ignoreRChanges); | ||
} else { | ||
gateEncoder = std::make_shared<SingleGateEncoder>( | ||
N, S, T, tableauEncoder->getVariables(), lb, config.gateSet, config.ignoreRChanges); | ||
N, S, T, tableauEncoder->getVariables(), lb, config.gateSet, | ||
config.ignoreRChanges); | ||
} | ||
gateEncoder->createSingleQubitGateVariables(); | ||
gateEncoder->createTwoQubitGateVariables(); | ||
|
@@ -81,7 +84,7 @@ void SATEncoder::createFormulation() { | |
} | ||
|
||
objectiveEncoder = std::make_shared<ObjectiveEncoder>( | ||
N, T, gateEncoder->getVariables(), lb, config.gateSet); | ||
N, T, gateEncoder->getVariables(), lb, config.gateSet); | ||
|
||
if (config.gateLimit.has_value()) { | ||
objectiveEncoder->limitGateCount(*config.gateLimit, std::less_equal{}); | ||
|
@@ -121,31 +124,32 @@ void SATEncoder::extractResultsFromModel(Results& res) const { | |
tableauEncoder->extractTableauFromModel(res, T, *model); | ||
auto qc = gateEncoder->extractCircuitFromModel(res, *model); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This might create an unnecessary copy of the quantum circuit in case |
||
if (config.ignoreRChanges) { | ||
const auto start = std::chrono::high_resolution_clock::now(); | ||
auto tab = *config.targetTableau; | ||
auto qcTab = Tableau(qc, 0, std::numeric_limits<std::size_t>::max(), config.targetTableau->hasDestabilizers()); | ||
for(std::size_t row = 0U; row < S; ++row) { | ||
DEBUG() << "Row " << std::to_string(row); | ||
tab[row][2*N] = qcTab.at(row)[2*N]; | ||
} | ||
const auto start = std::chrono::high_resolution_clock::now(); | ||
auto tab = *config.targetTableau; | ||
auto qcTab = Tableau(qc, 0, std::numeric_limits<std::size_t>::max(), | ||
config.targetTableau->hasDestabilizers()); | ||
Comment on lines
+134
to
+135
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you were to implement the change proposed above, I would guess that this can be skipped completely as the resulting tableau from the circuit is available in the results. |
||
for (std::size_t row = 0U; row < S; ++row) { | ||
DEBUG() << "Row " << std::to_string(row); | ||
tab[row][2 * N] = qcTab.at(row)[2 * N]; | ||
} | ||
PhaseCorrectionEncoder phaseCorrectionEncoder(N, S, tab, | ||
*config.targetTableau); | ||
auto paulis = phaseCorrectionEncoder.phaseCorrection(); | ||
for(std::size_t row = 0U; row < S; ++row) { | ||
tab[row][2*N] = config.targetTableau->at(row)[2*N]; | ||
auto paulis = phaseCorrectionEncoder.phaseCorrection(); | ||
|
||
for (std::size_t row = 0U; row < S; ++row) { | ||
tab[row][2 * N] = config.targetTableau->at(row)[2 * N]; | ||
} | ||
|
||
for (std::size_t q = 0U; q < N; ++q) { | ||
if (paulis[q] != qc::OpType::None) { | ||
DEBUG() << "Phase correction for qubit " << q << ": " | ||
<< qc::toString(paulis[q]); | ||
qc.emplace_back<qc::StandardOperation>(N, q, paulis[q]); | ||
} | ||
qc.emplace_back<qc::StandardOperation>(N, q, paulis[q]); | ||
} | ||
} | ||
const auto end = std::chrono::high_resolution_clock::now(); | ||
const auto runtime = std::chrono::duration<double>(end - start); | ||
res.setRuntime(res.getRuntime() + runtime.count()); | ||
const auto end = std::chrono::high_resolution_clock::now(); | ||
const auto runtime = std::chrono::duration<double>(end - start); | ||
res.setRuntime(res.getRuntime() + runtime.count()); | ||
res.setResultCircuit(qc); | ||
res.setResultTableau(tab); | ||
res.setDepth(qc.getDepth()); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. All of the new options are missing from the Python stubs for QMAP. Please also add any additions to the bindings there. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -527,8 +527,8 @@ PYBIND11_MODULE(pyqmap, m) { | |
"Gate Set to be used for the Synthesis. " | ||
"Defaults to {H, S, Sdg, X, Y, Z, CX}.") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This comment is not really true. |
||
.def_readwrite("delay_paulis", &cs::Configuration::delayPaulis, | ||
"Delay Pauli gates to the end of the circuit. " | ||
"Defaults to `false`.") | ||
"Delay Pauli gates to the end of the circuit. " | ||
"Defaults to `false`.") | ||
Comment on lines
+530
to
+531
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe it would be good to add a statement here that comments on whether this preserves optimality. |
||
.def("json", &cs::Configuration::json, | ||
"Returns a JSON-style dictionary of all the information present in " | ||
"the :class:`.Configuration`") | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am surprised by the lack of further tests asserting the proper behaviour given the size of the changes in this PR. |
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.
Heads-up: I personally despise
Utils
files, so please feel free to disagree on this, but I do not think the functionality here should be in a separate file.All of this can simply go into the
GateSet
class/file. TheisPauli
method can even be used there to simplify some if condition.