C++ Simulator Refactor #386

chriseclectic · 2018-04-04T15:46:08Z

This PR is a rebase of the C++ code changes from PR #351, with the python code changes for backend renaming to be split into a seperate PR.

Description

Added unit tests for C++ simulator
Added TensorIndex C++ class (tensor_index.hpp) for multi-partite qubit vector indexing
Added QubitVector C++ class (qubit_vector.hpp) for multi-partite qubit vector algebra
Reworked C++ simulator backends to use QubitVector class and methods instead of std::vector.
Added snapshot command for simulator for caching a copy of the current simulator state and returning in the output
Added snapshot circuit extension as Gate (will introduce pragmas after this release)
Removed the ability to return the cached states from the save command (use snapshot instead)
Removed the ability to return the final quantum state of the simulator (use snapshot instead)
renamed simulator folder to src/qasm-simulator-cpp, renamed executable to qasm_simulator_cpp.

Motivation and Context

This refactor fixes some bugs in the C++ simulator, includes unit tests, and allows for easier linking of the simulator later to make libraries (such as with cython).

How Has This Been Tested?

All current tests pass
New tests (both python unit tests and direct C++ tests) have been added for the simulator

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have read the CONTRIBUTING document.
I have added tests to cover my changes.
All new and existing tests passed.

atilag

Ready to merge! I think @chriseclectic still needs to update the branch with a fix for the SDK to find the simulator into the build dir.

atilag · 2018-04-11T14:39:00Z

src/qasm-simulator-cpp/src/Makefile

+WARNINGS = -pedantic -Wall -Wextra -Wfloat-equal -Wundef -Wcast-align -Wwrite-strings -Wmissing-declarations -Wredundant-decls -Wshadow -Woverloaded-virtual
+OPT = -O3 -march=native -ffast-math
+# Install exe into pip location for backends
+OUTPUT_DIR = ../../../qiskit/backends


"out of source compilation" is a very common practice, where the results of a build are created in an isolated and separated from the sources directory. I'd recommend set the OUTPUT_DIR to ../../../out
We only use qiskit/backeds/ as a workaround when packaging for distributing with pip, but this is going to disappear once we have the library interface for the simulator ready (it will end-up being a library so installed with the rest of the native libraries).

atilag · 2018-04-11T14:41:14Z

src/qasm-simulator-cpp/src/Makefile

+	$(CC) $(CPPFLAGS) $(DEFINES) -o ${OUTPUT_DIR}/qasm_simulator_cpp main.o $(LIBS)
+
+sim_debug: main.o
+	$(CC) -g $(CPPFLAGS) $(DEFINES) -o ${OUTPUT_DIR}/qasm_simulator_cpp_debug main.o $(LIBS)


Now that we have Release and Debug builds, I'd suggest using ${OUTPUT_DIR}/Release/ and ${OUTPUT_DIR}/Debug/ so there's no need to rename the executable.

atilag · 2018-04-11T15:24:24Z

src/qasm-simulator-cpp/src/backends/base_backend.hpp

    if (n > 0)
      omp_threads = n;
  };
-  inline void set_omp_threshold(int_t n) {
+  inline void set_omp_threshold(int n) {


Seems like uint32_t (or uint8_t?) might be a better type.

atilag · 2018-04-12T09:41:42Z

src/qasm-simulator-cpp/src/simulator.hpp

-    Backend backend = circ.config;
+    Backend backend;
+    backend.set_config(circ.config);
+    backend.attach_noise(circ.noise);

    // Set RNG Seed
    uint_t rng_seed = (circ.rng_seed < 0) ? std::random_device()()


This is always false, as circ.rng_seed is initialized to -1 in Circuit.hpp but never changed again. Not a big deal probably, mentioning just in case.

Yep, by default it will do this, but it can be override at runtime from the seed setting in the input file

chriseclectic · 2018-04-12T17:12:21Z

@atilag @ajavadia Only a couple of things left to do:

Figure out why travis tests can't find the simulator executable
Fix CMake for Xcode static linking (or at least add instructions for workaround)
Update src/qasm-simulator-cpp/README.md for CMake build instructions (including above for Xcode, and using Homebrew GCC on Mac to get OpenMP support)
Double check against Ali's PR to see if any of the C++ changes accidentally got left there

diego-plan9 · 2018-04-12T19:05:14Z

For the travis issue - can you make the tests that require an instance of the simulator to be skipped? This is currently the behaviour in master, and I believe we should have the same approach until we reach the point where the simulator is guaranteed to be available in all platforms and environments. The first stage of travis ("lint and pure python test") does not build the non-python code in purpose - skipping the tests would make it pass (and then proceed with the full testing suit on the second stage 🤞 ).

ajavadia · 2018-04-13T04:52:04Z

src/qasm-simulator-cpp/src/third-party/headers/json.hpp

+{
+    if (JSON_UNLIKELY(not j.is_string()))
+    {
+        JSON_THROW(type_error::create(302, "type must be string, but is " + std::string(j.type_name())));


@chriseclectic The JSON parser used here expects all identifiers for QuantumRegister, ClassicalRegister, QuantumCircuit to be strings. However, in #308 supporting non-string identifiers was introduced. Is it possible to allow that here?

I realize that it may be hard, in which case we can skip it for now. The reason I came across this was in the context of allowing interchangeability of simulators. There are 3 tests in test_identifiers.py which will pass with the python simulator but not the c++ simulator. All 3 are named test_add_circuit.

diego-plan9 · 2018-04-13T10:35:16Z

Can we take the chance to reintroduce the travis osx builds before merging (basically, uncommenting the changes done at #374? They were disabled as travis was having issues during those days that seem to be recovered by now.

atilag · 2018-04-13T10:59:18Z

Yeah, I got it.

chriseclectic · 2018-04-13T18:42:51Z

@atilag I've added everything I need for the merge

- Updated modified JSON library to version 3.1.1 - Encapsulated multi-partite qubit state vector updates in a QubitVector class - Added TensorIndex class used for indexing in the QubitVector class. - Reworked ideal_backend and qubit_backend to use QubitVector methods - merged sampleshots_engine into vector_engine - removed ability to display final and saved quantum states - added “snapshot” instruction and added ability to display snapshots of quantum state - added basic cpp test file for qubit_vector class - Added auto check to makefile for GCC7 compiler on macOS

- added string simulator input/output - Fixed compile bugs on linux - changed snapshot map to use int key instead of unsigned long long key - allowed sample shots to compute probability vector in place to save memory

…uild

…erialisation of clifford snapshots

cmake build needs updating for new directory / exe file name

renamed _local_qiskit_simulator.py to _local_qasm_simulator_cpp.py

…c tests

…tionals

* Makefile: output build directory changed * Makefile fixes for out-of-source compilation

If the binary is not found, then the test will skip in stage1, on later stages, there's a build step that will make travis fail if something is wrong.

* new dependencies target (make depend) for the qasm-simulator-cpp (cherry picked from commit 3b25390) * changed to executable and bug fix (cherry picked from commit ce8be89) * assume yes for automatic pkg install (cherry picked from commit 2cd7607) * -march not supported on ppc64le. Use -mcpu instead (cherry picked from commit 7cdab8f) * bugfix (cherry picked from commit 831692e)

Increase shots to fix failing test cleaning up some simulator variable names

chriseclectic · 2018-04-13T20:18:56Z

I rebased against master and squashed a few commits

C++ Simulator Refactor

chriseclectic requested review from atilag and diego-plan9 April 4, 2018 15:46

chriseclectic force-pushed the cpp-refactor branch from 255d024 to 616826f Compare April 4, 2018 16:12

chriseclectic mentioned this pull request Apr 4, 2018

[WIP] Cython based building and linking of C++ simulator #387

Closed

chriseclectic force-pushed the cpp-refactor branch from 616826f to 3a2dbde Compare April 4, 2018 17:28

chriseclectic mentioned this pull request Apr 6, 2018

[WIP] Simulator backends refactor #351

Closed

3 tasks

atilag approved these changes Apr 12, 2018

View reviewed changes

ajavadia reviewed Apr 13, 2018

View reviewed changes

ajavadia approved these changes Apr 13, 2018

View reviewed changes

chriseclectic and others added 16 commits April 13, 2018 16:13

Improved handling of JSON for cpp simulator

ad649e2

- added string simulator input/output - Fixed compile bugs on linux - changed snapshot map to use int key instead of unsigned long long key - allowed sample shots to compute probability vector in place to save memory

Make qubit_vector compatible with older OpenMP 2.0 spec for windows b…

bc4e077

…uild

Updated makefile to install to qiskit/backends, fix error in JSON des…

16486e1

…erialisation of clifford snapshots

renamed qiskit-simulator folder to qasm-simulator-cpp

b9e7aad

cmake build needs updating for new directory / exe file name

- added default qubit, clbit values to qobj circuit header

1a70ea2

renamed added tests for qasm_simulator_cpp

aff6625

renamed _local_qiskit_simulator.py to _local_qasm_simulator_cpp.py

renamed qiskit_simulator extensions to qasm_simulator_cpp, added basi…

957cc17

…c tests

add cpp conditional test qobj, improved hexadecimal parsing for condi…

3c53764

…tionals

* Fixing CMake build

6035fb6

* Makefile: output build directory changed * Makefile fixes for out-of-source compilation

Modernizing C++ code to new standards

6374a4c

use os.path.join for combining default simulator exe paths

c2b6142

Disable simulator tests so the stage1 of Travis can succeed.

ee08d1b

If the binary is not found, then the test will skip in stage1, on later stages, there's a build step that will make travis fail if something is wrong.

updated readme with cmake instructions

281dbf9

Enable MacOS travis tests

4b27671

Fixing OpenMP config settings

93b3092

Increase shots to fix failing test cleaning up some simulator variable names

chriseclectic force-pushed the cpp-refactor branch from 0a3a299 to 93b3092 Compare April 13, 2018 20:17

atilag merged commit 7066f7f into Qiskit:master Apr 14, 2018

jaygambetta mentioned this pull request Apr 18, 2018

QISKit 0.5 status and notes #413

Closed

77 tasks

chriseclectic deleted the cpp-refactor branch September 14, 2018 21:08

lia-approves pushed a commit to edasgupta/qiskit-terra that referenced this pull request Jul 30, 2019

Merge pull request Qiskit#386 from chriseclectic/cpp-refactor

90eb2dc

C++ Simulator Refactor

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

C++ Simulator Refactor #386

C++ Simulator Refactor #386

chriseclectic commented Apr 4, 2018

atilag left a comment

atilag Apr 11, 2018

atilag Apr 11, 2018

atilag Apr 11, 2018

atilag Apr 12, 2018

chriseclectic Apr 12, 2018

chriseclectic commented Apr 12, 2018 •

edited

Loading

diego-plan9 commented Apr 12, 2018

ajavadia Apr 13, 2018

diego-plan9 commented Apr 13, 2018

atilag commented Apr 13, 2018

chriseclectic commented Apr 13, 2018

chriseclectic commented Apr 13, 2018

C++ Simulator Refactor #386

C++ Simulator Refactor #386

Conversation

chriseclectic commented Apr 4, 2018

Description

Motivation and Context

How Has This Been Tested?

Types of changes

Checklist:

atilag left a comment

Choose a reason for hiding this comment

atilag Apr 11, 2018

Choose a reason for hiding this comment

atilag Apr 11, 2018

Choose a reason for hiding this comment

atilag Apr 11, 2018

Choose a reason for hiding this comment

atilag Apr 12, 2018

Choose a reason for hiding this comment

chriseclectic Apr 12, 2018

Choose a reason for hiding this comment

chriseclectic commented Apr 12, 2018 • edited Loading

diego-plan9 commented Apr 12, 2018

ajavadia Apr 13, 2018

Choose a reason for hiding this comment

diego-plan9 commented Apr 13, 2018

atilag commented Apr 13, 2018

chriseclectic commented Apr 13, 2018

chriseclectic commented Apr 13, 2018

chriseclectic commented Apr 12, 2018 •

edited

Loading