MGARD-X portable implementation

[JieyangChen7]

@ben-e-whitney @qliu21 Do you guys know what causes this Doxygen error: Compound XXX is not documented. (warning treated as error, aborting now)?

[ben-e-whitney]

It's just complaining that a class isn't documented. In the meeting I suggested that we make the following change to .doxygen.in.

diff --git a/.doxygen.in b/.doxygen.in
index 4aac421..15d2ae1 100644
--- a/.doxygen.in
+++ b/.doxygen.in
@@ -27,7 +27,7 @@ GENERATE_DEPRECATEDLIST= YES
 # Configuration options related to warning and progress messages
 #---------------------------------------------------------------------------

-WARN_AS_ERROR          = YES
+WARN_AS_ERROR          = NO

 #---------------------------------------------------------------------------
 # Configuration options related to the input files

With that, though, we get quite a lot of errors about undocumented things. I suggest we make the following change instead.

diff --git a/.doxygen.in b/.doxygen.in
index 4aac421..6bdb7ca 100644
--- a/.doxygen.in
+++ b/.doxygen.in
@@ -36,7 +36,7 @@ WARN_AS_ERROR          = YES
 INPUT                  = include @MGARD_DOC_PAGES_JOINED@
 RECURSIVE              = YES
 FILE_PATTERNS          = *.h *.hpp
-EXCLUDE                = include/cuda @MGARD_EXECUTABLE_HPP_JOINED@
+EXCLUDE                = include/cuda include/mgard-x @MGARD_EXECUTABLE_HPP_JOINED@
 USE_MDFILE_AS_MAINPAGE = @MGARD_DOC_MAINPAGE@

 #---------------------------------------------------------------------------

Then Doxygen will enforce documentation of the old code and skip the new code. See if the check passes once you make that change. We can unexclude include/mgard-x later if we want to document all those headers.

[ben-e-whitney]

These commits add 103 CSV files and 16 PNG files. They appear to be benchmarking results. Just checking that they're meant to be included.

$ git diff --diff-filter=A --name-only master HEAD | grep '\.csv$' | head
src/mgard-x/Testing/MDR/encoding_perf_results/pref_32_32_1024_32_32_0_1_1_1.csv
src/mgard-x/Testing/MDR/encoding_perf_results/pref_32_32_1024_32_32_1_1_1_1.csv
src/mgard-x/Testing/MDR/encoding_perf_results/pref_32_32_1048576_32_32_0_1_1_1.csv
src/mgard-x/Testing/MDR/encoding_perf_results/pref_32_32_1048576_32_32_1_1_1_1.csv
src/mgard-x/Testing/MDR/encoding_perf_results/pref_32_32_131072_32_32_0_1_1_1.csv
src/mgard-x/Testing/MDR/encoding_perf_results/pref_32_32_131072_32_32_1_1_1_1.csv
src/mgard-x/Testing/MDR/encoding_perf_results/pref_32_32_134217728_32_32_0_1_1_1.csv
src/mgard-x/Testing/MDR/encoding_perf_results/pref_32_32_134217728_32_32_1_1_1_1.csv
src/mgard-x/Testing/MDR/encoding_perf_results/pref_32_32_16384_32_32_0_1_1_1.csv
src/mgard-x/Testing/MDR/encoding_perf_results/pref_32_32_16384_32_32_1_1_1_1.csv
$ git diff --diff-filter=A --name-only master HEAD | grep '\.png$'
examples/mgard-x/Evaluation/E3SM.png
examples/mgard-x/Evaluation/E3SM_CR.png
examples/mgard-x/Evaluation/NYX.png
examples/mgard-x/Evaluation/NYX_CR.png
examples/mgard-x/Evaluation/XGC.png
examples/mgard-x/Evaluation/XGC_CR.png
src/mgard-x/Testing/MDR/decoding_data_sizes_binarytype_0.png
src/mgard-x/Testing/MDR/decoding_data_sizes_binarytype_1.png
src/mgard-x/Testing/MDR/decoding_num_bitplanes_binarytype_0.png
src/mgard-x/Testing/MDR/decoding_num_bitplanes_binarytype_1.png
src/mgard-x/Testing/MDR/encoding_data_sizes_binarytype_0.png
src/mgard-x/Testing/MDR/encoding_data_sizes_binarytype_1.png
src/mgard-x/Testing/MDR/encoding_num_bitplanes_binarytype_0.png
src/mgard-x/Testing/MDR/encoding_num_bitplanes_binarytype_1.png
src/mgard-x/Testing/MDR/reconstuct.png
src/mgard-x/Testing/MDR/refactor.png

[ben-e-whitney]

The following change to CMakeLists.txt, made in fe8fccbd810c67ceef44204e0ee1a4b689359be5, breaks my build.

@@ -177,7 +178,8 @@ find_package(ZLIB REQUIRED)

 find_package(PkgConfig)

-pkg_search_module(ZSTD IMPORTED_TARGET GLOBAL libzstd)
+# pkg_search_module(ZSTD IMPORTED_TARGET GLOBAL libzstd)
+find_package(zstd)

 find_package(MOAB)
 #We run into this when building MOAB 5.3.0 with `./configure && make`.

Here's what happens:

$ cmake -S . -B "build" 1>/dev/null
CMake Warning at CMakeLists.txt:182 (find_package):
  By not providing "Findzstd.cmake" in CMAKE_MODULE_PATH this project has
  asked CMake to find a package configuration file provided by "zstd", but
  CMake did not find one.

  Could not find a package configuration file provided by "zstd" with any of
  the following names:

    zstdConfig.cmake
    zstd-config.cmake

  Add the installation prefix of "zstd" to CMAKE_PREFIX_PATH or set
  "zstd_DIR" to a directory containing one of the above files.  If "zstd"
  provides a separate development package or SDK, be sure it has been
  installed.

This is because my Zstandard installation provides libzstd.pc but not zstdConfig.cmake/zstd-config.cmake.

$ dpkg --listfiles libzstd-dev | grep '\.\(pc\|cmake\)$'
/usr/lib/x86_64-linux-gnu/pkgconfig/libzstd.pc

I will figure out how to get this working after we merge.

Incidentally, Jieyang, I cannot believe how enormous this pull request is. They should be paying you more. Great job.

[ben-e-whitney]

@JieyangChen7, it looks like you need to document a few things in include/compress_x.hpp and then the docs check will pass. Also, I would bump MGARD_VERSION_MINOR in CMakeLists.txt because this adds (a lot of) new functionality.

diff --git a/CMakeLists.txt b/CMakeLists.txt
index f18de95..ca481d5 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -11,7 +11,7 @@ endif()
 list(INSERT CMAKE_MODULE_PATH 0 "${CMAKE_CURRENT_LIST_DIR}/cmake")

 set(MGARD_VERSION_MAJOR "1")
-set(MGARD_VERSION_MINOR "1")
+set(MGARD_VERSION_MINOR "2")
 set(MGARD_VERSION_PATCH "0")

 set(MGARD_FILE_VERSION_MAJOR "1")

[JieyangChen7]

@ben-e-whitney Thanks for taking the time to review my pull request.

About the CSV files, yes, they are profiling/benchmark results. I meant to keep them in there since I think it would be useful to show some profiling results that we obtained on the readme (or user guide) so that users know what to expect when they use MGARD with a similar dataset and/or on a similar machine. The current readme (or user guide) does not include those yet. I will try to include those in the next pull request. Also, I will try to keep the number of CSV files as small as possible to void increase the size of the software by too much.

About Zstd, I made that change because I want to force MGARD to link with the shared library version of Zstd, since by default multithreading is enabled in the shared library and disabled in the static library (see here). Of course, we can always force multithreading to be enabled for both shared and static libraries, but I think that will add an extra burden to the users. I know how to link with the shared library using target_link_libraries(mgard-library PUBLIC zstd::libzstd_shared), but I'm not sure how to do that when a library is imported by pkg-config (I'm not very familiar with it). BTW, if you build Zstd with CMake, it will generate the zstdConfig.cmake file. I have provided the script for building Zstd with CMake in here for users.

Please feel free to let me know if you have any suggestions on those issues. Thanks again!

[JieyangChen7]

@qliu21 I think it can pass all checks and is ready to be merged.