Support multiple input/output states. #270

s-mandra · 2020-02-04T18:28:24Z

Improve code to handle multiple initial/final states.

src/evaluate_circuit.h

src/main.cpp

src/pybind_main.cpp

src/read_circuit.h

src/pybind_main.cpp

95-martin-orion

First pass.

95-martin-orion · 2020-02-05T16:48:58Z

src/evaluate_circuit.h

@@ -58,12 +62,33 @@ std::vector<std::vector<std::size_t>> read_grid_layout_from_stream(
 * be populated by this method.
 * @param output_states vector of output states for the given contraction


Update comment (output_states is no longer a parameter)

95-martin-orion · 2020-02-05T16:51:05Z

src/evaluate_circuit.h

+/**
+ * Given a grid of 3D tensors, rename indexes of the terminal tensors to
+ * reflect terminal qubits.
+ * @param QflexFinalQubits representing the final qubits.


For specifying parameters in function comments, please use the format:

* @param <parameter_name> <parameter_description>

Also, it looks like some parameters are missing from this comment.

95-martin-orion · 2020-02-05T16:51:15Z

src/evaluate_circuit.h

+
+/**
+ * Apply the final delta-tensors to the 3D grid of tensors.
+ * @param QflexGrid representing the circuit layout.


See previous comment.

95-martin-orion · 2020-02-05T16:52:41Z

src/evaluate_circuit.h

@@ -78,8 +103,8 @@ std::string get_output_states(
 * States for qubits with terminal cuts are listed at the end of the state
 * bitstring, in the order of their terminal cuts.
 */
-std::vector<std::pair<std::string, std::complex<double>>> EvaluateCircuit(
-    QflexInput* input);
+std::vector<std::tuple<std::string, std::string, std::complex<double>>>


With this change, the @return spec is no longer accurate - please update.

95-martin-orion · 2020-02-05T16:55:54Z

src/main.cpp

@@ -32,8 +32,8 @@ tensor network, CPU-based simulator of large quantum circuits.
    qflex::utils::readable_memory_string(qflex::global::memory_limit), R"(].
    -t,--track-memory=<seconds>            If <verbosity_level> > 0, track memory usage [default: )",
    qflex::global::track_memory_seconds, R"(].
-    --initial-conf=<initial_conf>          Initial configuration.
-    --final-conf=<final_conf>              Final configuration.
+    --initial-conf=<initial_conf>          Initial configuration [default: 00...00].


This help message should indicate that multiple input/output strings are supported, and (briefly) describe how qFlex handles them.

95-martin-orion

I think I still need to review evaluate_circuit.cpp, but this should cover the rest.

95-martin-orion · 2020-02-05T17:13:09Z

src/main.cpp

+        }
+      }
+
+    // Delete duplicate initial/final configuration


I'd actually suggest that we return an error if duplicates are detected. The reasoning is this: if a user manually specifies two input states, they probably meant to provide two different input states. Since a simulation run can take a long time, we should fail early so the user can correct their mistake.

95-martin-orion · 2020-02-05T17:16:10Z

src/pybind_main.cpp

-        amplitudes.push_back({is, EvaluateCircuit(&input)});
-      }
-    }
+    // Remove duplicate initial/final states


Same as above - duplicate states should produce an error.

95-martin-orion · 2020-02-05T17:21:23Z

src/read_circuit.cpp

@@ -320,18 +318,10 @@ void circuit_data_to_tensor_network(
  }

  // Check for the length of initial_conf and final_conf.


Update comment.

95-martin-orion · 2020-02-05T18:15:52Z

src/read_circuit.h

@@ -49,6 +49,15 @@ namespace qflex {
 const std::size_t SUPER_CYCLE_DEPTH = 8;
 const std::size_t DIM = 2;

+/**
+ * Return gate as an array.
+ * @param name of the gate.


@param formatting.

95-martin-orion · 2020-02-05T18:17:05Z

src/read_circuit.h

+ */
+std::vector<s_type> gate_array(const std::string& gate_name,
+                               const std::vector<double>& params);
+
 /**
 * Read circuit from stream and fill in a 2D grid of vectors of tensors.
 * @param qflex::QflexCircuit containing circuit information.


Update function comment to reflect parameter changes. (Also looks like the first @param is missing its parameter name.)

95-martin-orion · 2020-02-05T18:17:38Z

src/read_circuit.h

@@ -94,9 +101,13 @@ void circuit_data_to_tensor_network(
 * @param scratch pointer to s_type array with enough space for all scratch
 * work.
 */
-void flatten_grid_of_tensors(
+std::vector<std::vector<Tensor>> flatten_grid_of_tensors(


Update function comment.

95-martin-orion · 2020-02-05T18:19:37Z

tests/src/evaluate_circuit_test.cpp

@@ -9,15 +9,18 @@ namespace {
 class GetOutputStatesTest : public testing::Test {


File comment: could you add tests for multiple-(input/output) and the new functions (apply_terminal_cuts and apply_delta_output)?

95-martin-orion · 2020-02-05T18:27:15Z

tests/src/read_circuit_test.cpp

+                     tensor_grid_3D, &tensor_grid_2D, &scratch);
+
+  // Reorder the 2D grid
+  reorder_grid_of_tensors(&tensor_grid_2D, final_qubits.qubits,


Would a separate test for reorder_grid_of_tensors make sense, or does this provide enough coverage as-is?

dagart added 2 commits February 4, 2020 10:27

Support multiple input/output states.

08a69d5

Fix Stopwatch call. Improve log.

98cb72d

s-mandra commented Feb 4, 2020

View reviewed changes

src/evaluate_circuit.h Show resolved Hide resolved

src/evaluate_circuit.h Show resolved Hide resolved

src/evaluate_circuit.h Show resolved Hide resolved

src/main.cpp Show resolved Hide resolved

src/pybind_main.cpp Show resolved Hide resolved

src/read_circuit.h Show resolved Hide resolved

s-mandra self-assigned this Feb 4, 2020

s-mandra commented Feb 4, 2020

View reviewed changes

src/pybind_main.cpp Show resolved Hide resolved

dagart added 5 commits February 5, 2020 08:17

Add comment.

ff63866

Remove unused function signature.

d5dcc56

Add comment.

0261a61

Fix format.

80d8800

Add comment.

467bfaf

s-mandra requested a review from 95-martin-orion February 5, 2020 16:31

s-mandra marked this pull request as ready for review February 5, 2020 16:31

95-martin-orion requested changes Feb 5, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support multiple input/output states. #270

Support multiple input/output states. #270

s-mandra commented Feb 4, 2020

95-martin-orion left a comment

95-martin-orion Feb 5, 2020

95-martin-orion Feb 5, 2020

95-martin-orion Feb 5, 2020

95-martin-orion Feb 5, 2020

95-martin-orion Feb 5, 2020

95-martin-orion left a comment

95-martin-orion Feb 5, 2020

95-martin-orion Feb 5, 2020

95-martin-orion Feb 5, 2020

95-martin-orion Feb 5, 2020

95-martin-orion Feb 5, 2020

95-martin-orion Feb 5, 2020

95-martin-orion Feb 5, 2020

95-martin-orion Feb 5, 2020

		@@ -58,12 +62,33 @@ std::vector<std::vector<std::size_t>> read_grid_layout_from_stream(
		* be populated by this method.
		* @param output_states vector of output states for the given contraction

		@@ -320,18 +318,10 @@ void circuit_data_to_tensor_network(
		}

		// Check for the length of initial_conf and final_conf.

		@@ -9,15 +9,18 @@ namespace {
		class GetOutputStatesTest : public testing::Test {

Support multiple input/output states. #270

Are you sure you want to change the base?

Support multiple input/output states. #270

Conversation

s-mandra commented Feb 4, 2020

95-martin-orion left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

95-martin-orion left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment