Getting cppsim to compile from sources alongside the MindQuantum sources by dmikushin · Pull Request #23 · Takishima/mindquantum

dmikushin · 2022-12-20T10:29:47Z

In principle, cppsim source is now integrated into MQ, and compiles together with it. There are several items to be addressed yet:

The cmake and cmake/commands modules folder should not be reached with ../../.., this is ugly. But I was not able to find an example of a better way to do it in other subfolders.

include("${CMAKE_CURRENT_SOURCE_DIR}/../../../cmake/compiler_has_std_filesystem.cmake")

Accessing ../../include/cppsim include directory is also ugly, maybe there is a good environment variable to use instead?

CppSim still deals with dlfcn-win32 by itself, while all other dependencies are brought in globally, by MQ. I guess we should move this dependency out on MQ likewise, but before that maybe it's better to use cross-platform dynalo instead?
There is an issue with std::allocator, compiler complains about max_size(). Even though throw() version is not deprecated, it fails either way. Maybe we disable exceptions?

    // class std::allocator<std::complex<double> >’ has no member named ‘max_size’; did you mean ‘_M_max_size’?
    size_type max_size() const noexcept
    {
        std::allocator<T> a;
        return a.max_size();
    }

…to automatic arrays type deduction for v and tmp

… equal results

…arlier than the use of the parsed arguments

…n native module compilation

…n, based on the KEYWORD option

…ng string of source code. We intentionally keep the generator in Python, in order to let the people to customize it more easily.

… rebuild after resource modification

…n to the kernels

…generated kernel to the correctness test. Breaking compatibility with the original kernels API, in order to be able to supply psi and m as plain pointers

…ompiled kernel

…ime compilation

…pilation. On the other hand, we could possibly use arrays for matrix operations?

…should be able to generated vectorized matrix multiplies (given that the compiler options are set). Matrix-vector itself is not yet efficient though, moreover the gaps in 'psi' elements are bad for memory efficiency

… The compilation times with eigen are an order of magnitude faster now, feels like the time is flat

Add a check to make sure the kernelgen.py script does exists based on the input to the kernelgen() function. Also make kernelgen() a function instead of a macro and add a `COMBINATIONS` arguments

…l file

… MSVC

…ome Cygwin/MinGW issues

…nfigs, which somehow fail to advertise themselves as MSYS or MINGW environments

…ve without it everywhere else

Takishima · 2022-12-21T11:11:02Z

1. The `cmake` and `cmake/commands` modules folder should not be reached with `../../..`, this is ugly. But I was not able to find an example of a better way to do it in other subfolders.

This is easily fixed. As long as the corresponding folders are inside the CMAKE_MODULE_PATH you can include them without using a path:

include(compiler_has_std_filesystem)

2. CppSim still deals with `dlfcn-win32` by itself, while all other dependencies are brought in globally, by MQ. I guess we should move this dependency out on MQ likewise, but before that maybe it's better to use cross-platform dynalo instead?

Sure we could do that. Otherwise, we can just add dlfcn-win32 to the list of third parties in MindQuantum and only load it when necessary.

3. There is an issue with std::allocator, compiler complains about max_size(). Even though throw() version is not deprecated, it fails either way. Maybe we disable exceptions?

As discussed yesterday, look at the version included in HiQ-ProjectQ.

Takishima

Here's a preliminary review of the code with some comments. More work is needed before we can propose this for a merge.

Also, the source code is missing the license information which might trigger some tools once we do perform the actual merge on Gitee.

Takishima · 2022-12-21T11:13:02Z

+cmake_minimum_required(VERSION 3.20 FATAL_ERROR)
+
+project(
+  cppsim
+  VERSION 1.0.0
+  LANGUAGES C CXX)
+
+# Minimum required Python version (used both in this file and in the installed CMake configuration)
+set(CPPSIM_PYTHON_VERSION_MIN 3.7.0)
+
+option(BUILD_TESTING "Build the test suite?" OFF)
+
+if(NOT DEFINED ITERATOR_DEBUG_VALUE)
+  set(ITERATOR_DEBUG_VALUE 0)
+endif()
+option(MSVC_ITERATOR_DEBUG "Define _ITERATOR_DEBUG_LEVEL (defaults to 0, can be set using ITERATOR_DEBUG_VALUE)" OFF)


To match with the rest of MindQuantum, remove these lines. The CMake options and variables should be already defined by MindQuantum

Takishima · 2022-12-21T11:14:28Z

+include(GNUInstallDirs)
+
+set(CPPSIM_INSTALL_BINDIR "${CMAKE_INSTALL_BINDIR}")
+set(CPPSIM_INSTALL_SBINDIR "${CMAKE_INSTALL_SBINDIR}")
+set(CPPSIM_INSTALL_SYSCONFDIR "${CMAKE_INSTALL_SYSCONFDIR}")
+set(CPPSIM_INSTALL_INCLUDEDIR "${CMAKE_INSTALL_INCLUDEDIR}/cppsim")
+set(CPPSIM_INSTALL_DATADIR "${CMAKE_INSTALL_DATADIR}/cppsim")
+set(CPPSIM_INSTALL_LIBDIR "${CMAKE_INSTALL_LIBDIR}/cppsim")
+set(CPPSIM_INSTALL_DOCDIR "${CMAKE_INSTALL_DATADIR}/doc/cppsim")
+set(CPPSIM_INSTALL_CMAKEDIR "${CPPSIM_INSTALL_DATADIR}/cmake")
+set(CPPSIM_INSTALL_3RDPARTYDIR "${CPPSIM_INSTALL_LIBDIR}/third_party")


Modify those so that they are using the MQ_INSTALL_XXX CMake variables.

You may redefined the CPPSIM_INSTALL_XXX to be relative to MQ_INSTALL_3RDPARTYDIR for example

Takishima · 2022-12-21T11:15:35Z

+# ==============================================================================
+
+# TODO
+include("${CMAKE_CURRENT_SOURCE_DIR}/../../../cmake/compiler_has_std_filesystem.cmake")


As explained in another comment, you should be able to simply write

include(compiler_has_std_filesystem)

This should work since MindQuantum modifies the CMAKE_MODULE_PATH variables elsewhere for this exact purpose.

Takishima · 2022-12-21T11:16:36Z

+  if(NOT DEFINED Eigen3_DIR)
+    FetchContent_Declare(
+      Eigen3
+      GIT_REPOSITORY https://gitlab.com/libeigen/eigen.git
+      GIT_TAG 3147391d946bb4b6c68edd901f2add6ac1f31f8c)
+    FetchContent_MakeAvailable(googletest Eigen3)
+  else()
+    find_package(Eigen3 CONFIG REQUIRED)
+  endif()


This should be removed since Eigen3 is found by MindQuantum in its third-party libraries directory.

You can add a short comment saying that one could want to look for Eigen3 here if it was not before.

Takishima · 2022-12-21T11:19:15Z

@@ -0,0 +1,43 @@
+#include <algorithm>


Test code should go under tests

Takishima · 2022-12-21T11:19:33Z

@@ -0,0 +1,142 @@
+// Ensure hand-written and generated kernels give equal results.


Test code should go under tests

Takishima · 2022-12-21T11:19:40Z

@@ -0,0 +1,271 @@
+#include <algorithm>


Test code should go under tests

Takishima · 2022-12-21T11:23:21Z

+find_package(res_embed QUIET CONFIG)
+if(NOT res_embed_FOUND)
+  FetchContent_Declare(
+    res_embed
+    GIT_REPOSITORY https://github.com/dmikushin/res_embed.git
+    GIT_TAG 93b5711070086dea53c3b535018ff34e68479242)
+  FetchContent_MakeAvailable(res_embed)
+else()
+  message(STATUS "Found res_embed at ${res_embed_DIR}")
+endif()


Like for Eigen3 these lines are not required since ResEmbed is a third-party of MindQuantum.

You can add a short comment saying that one could want to look for ResEmbed here if it was not before.

Takishima · 2022-12-21T11:52:51Z

Also ensure that the versions of the third-party specified in the third_party folders are the same you are specifying (e.g. for ResEmbed)

dmikushin added 30 commits December 20, 2022 07:39

Working on the nointrin generator

63e3691

Working on the code generation for kernel_core

68915d9

Finalizing the code generation for kernel_core

48d245c

Adding build scripts, adding test program stub

6b30b41

Working on the test stub

7cc0b7a

Getting all 5 handwritten kernels to compile within the test, thanks …

2b67121

…to automatic arrays type deduction for v and tmp

Finalizing the test to ensure hand-written and generated kernels give…

c1928b8

… equal results

Adding class for creating temporary files

907733a

Embedding kernel generation script as a resource

a61040d

Correcting the wrong order of statements: agrument parser must come e…

dea7736

…arlier than the use of the parsed arguments

Adding the rest of the original _cppsim source code, adding the Pytho…

3d7087e

…n native module compilation

Adding support for both plain and keywork link libraries specificatio…

707f6fc

…n, based on the KEYWORD option

Working on the embedded kernel generation and a benchmark executable

2bfe527

Updating the README

3e37b96

Clarifying the README description

f5996f9

Use embedded Python interpreter to run the script and get the resulti…

6c352c5

…ng string of source code. We intentionally keep the generator in Python, in order to let the people to customize it more easily.

Adding intermediate files as dependencies, in order to perform proper…

befaca6

… rebuild after resource modification

Working on the runtime compilation support

b6f98b1

Getting runtime compilation to succeed

eaf5014

Breaking the API compatibility, in order to pass generic pointers dow…

908b26d

…n to the kernels

Removing debug print

a56e5c7

Getting the runtime-generated kernel executed, adding OpenMP, adding …

77151ab

…generated kernel to the correctness test. Breaking compatibility with the original kernels API, in order to be able to supply psi and m as plain pointers

Adding an option to substitute ids values directly into the runtime-c…

1275bee

…ompiled kernel

Working on a better representation of the generator script

ed4a7d2

Finalizing better representation of the generator script

1331e41

Use sorted 'd' values as scalars instead of an array, in case of runt…

5b131af

…ime compilation

Use 'v' values as scalars instead of an array, in case of runtime com…

d6c1054

…pilation. On the other hand, we could possibly use arrays for matrix operations?

Letting runtime compilation to use the system-provided eigen library.…

39af62c

… The compilation times with eigen are an order of magnitude faster now, feels like the time is flat

Working on adding combinations headers

e49fab7

Takishima and others added 27 commits December 20, 2022 07:39

Add kernelgen path check and turn kernelgen into function

b436119

Add a check to make sure the kernelgen.py script does exists based on the input to the kernelgen() function. Also make kernelgen() a function instead of a macro and add a `COMBINATIONS` arguments

Fix changes missing from last commit

bd79df8

Add <array> and <functional> to kernels.hpp

c1f5249

Improve CXX standard handling for all targets + add .cmake-format.yam…

f4fdf20

…l file

Add compatibility with Boost::Filesystem for older compilers

44e1084

Fix compilation error on MSVC due to OpenMP

522654a

Fix missed #include <filesystem> for compatibility with older compilers

ec2df8a

Use Python.Module instead of Python.Embed

0671955

Fix compilation issues on various compilers

1668736

Fix compilation issues linked to <unistd.h>

c6b871f

Locate pybind11 after looking for Python

f8e438d

Use Python module instead of embed

b952ff7

Support dlopen on Windows

7834aa2

Help CMake on macOS to locate OpenMP when using HomeBrew and MacPorts

e08caf7

Add GitHub workflows

6dfbb9f

More verbose output

6881132

Fix typo

5779e9c

Fix CMake configuration error due to dlfcn-win32

89b7dd5

Allow customization of the _ITERATOR_DEBUG define when compiling with…

84f0ff4

… MSVC

Use dlfcn-win32 also on MSYS

2a2bd33

Update res_embed version

a313ac9

Use dmikushin/res_embed for GitHub workflows

c8713ec

Bumping the res_embed GIT_TAG to a version, which (hopefully) fixes s…

c975c61

…ome Cygwin/MinGW issues

Do not use .att_mnemonic, which is not supported by Clang

b9bcccb

Ensuring NO_TYPE_FOR_PECOFF reaches MSYS2 MINGW64 and MSYS2 UCRT64 co…

7879110

…nfigs, which somehow fail to advertise themselves as MSYS or MINGW environments

Don't use .type directive unsupported on Windows at all, if we can li…

be87a2b

…ve without it everywhere else

Getting cppsim to compile from sources alongside the MindQuantum sources

7c43563

Takishima reviewed Dec 21, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Getting cppsim to compile from sources alongside the MindQuantum sources#23

Getting cppsim to compile from sources alongside the MindQuantum sources#23
dmikushin wants to merge 82 commits into
Takishima:feature/cppsimfrom
dmikushin:feature/cppsim

dmikushin commented Dec 20, 2022

Uh oh!

Takishima commented Dec 21, 2022

Uh oh!

Takishima left a comment

Uh oh!

Takishima Dec 21, 2022

Uh oh!

Takishima Dec 21, 2022

Uh oh!

Takishima Dec 21, 2022

Uh oh!

Takishima Dec 21, 2022

Uh oh!

Takishima Dec 21, 2022

Uh oh!

Takishima Dec 21, 2022

Uh oh!

Takishima Dec 21, 2022

Uh oh!

Takishima Dec 21, 2022

Uh oh!

Takishima commented Dec 21, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		@@ -0,0 +1,142 @@
		// Ensure hand-written and generated kernels give equal results.

Conversation

dmikushin commented Dec 20, 2022

Uh oh!

Takishima commented Dec 21, 2022

Uh oh!

Takishima left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Takishima commented Dec 21, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants