search

robertu94 / libpressio

A library to abstract between different lossless and lossy compressors
Other
30 stars 11 forks source link

modifying ftk plugins to use the simplified critical point data structure #12

Closed hguo closed 3 years ago

hguo commented 4 years ago

You can compile libpressio with the latest master branch in FTK. I have un-templated critical points to simplify the access to (discrete) critical points. I do not know how to test the correctness of the change. I also wonder how you are going to compare two sets of critical points.

robertu94 commented 4 years ago

@hguo Thanks for the pull request!

I should probably improve the documentation about how to test new metrics modules after they are written. I'll make a new issue for that. In general, you can either call libpressio using libpressio-tools and pass the -m ftk_critical_points -M all with your favorite 2d or 3d dataset to get the discrete critical points and see that it prints out that a metric was created. If you don't use libpressio-tools, you can write a small C/C++ file that is similar to ./test/metrics.c, but including ftk_critical_points in const char* metrics[]

Before we merge this there are a few changes I need to make:

As for comparing two sets of critical points, my goal is just to expose the set of critical points to the next "metric" up the hierarchy, and let the user decide how they want to compare them. In general, comparison of sets of critical points probably an open question with many possible solutions depending on the specific goal. Users could implement a small script in lua using the composite metrics module with LIBPRESSIO_HAS_LUA while we are experimenting, and we can codify the commonly used metrics into a metric plugin.

robertu94 commented 4 years ago

@hguo I have tried to build this and integrate this this morning, but I can no longer get libpressio to build with FTK. The errors come from functions that are no-longer defined that I don't directly reference in the ftk headers. I tried with both the master and dev branches. Could you please look into the fix?

[runderwood@voyager libpressio]$ m           
INFO:m.plugins.Base:running settings
INFO:m.plugins.Base:running build
INFO:m.plugins.Base:plugin CMakePlugin is DEFAULT_MAIN for build
m: Entering directory '/home/runderwood/git/libpressio/build'
[1/28] Building CXX object CMakeFiles/libpressio.dir/src/plugins/metrics/ftk_critical_points.cc.o
FAILED: CMakeFiles/libpressio.dir/src/plugins/metrics/ftk_critical_points.cc.o 
ccache /usr/bin/c++ -DFFTW3 -DFPZIP_FP=FPZIP_FP_FAST -Dlibpressio_EXPORTS -I../include -Iinclude -isystem /usr/include/sz -isystem /usr/include/ImageMagick-7 -isystem /usr/lib64/petsc/include -isystem /usr/include/luajit-2.0 -isystem /usr/include/libdistributed -g -fPIC -Wall -Werror -Wextra -Wpedantic -fopenmp -DMAGICKCORE_HDRI_ENABLE=0 -DMAGICKCORE_QUANTUM_DEPTH=16 -pthread -std=gnu++17 -MD -MT CMakeFiles/libpressio.dir/src/plugins/metrics/ftk_critical_points.cc.o -MF CMakeFiles/libpressio.dir/src/plugins/metrics/ftk_critical_points.cc.o.d -o CMakeFiles/libpressio.dir/src/plugins/metrics/ftk_critical_points.cc.o -c ../src/plugins/metrics/ftk_critical_points.cc
In file included from /usr/include/ftk/filters/critical_point_tracker.hh:6,
                 from /usr/include/ftk/filters/critical_point_tracker_regular.hh:6,
                 from ../src/plugins/metrics/ftk_critical_points.cc:5:
/usr/include/ftk/filters/critical_point.hh: In member function ‘std::ostream& ftk::critical_point_t::print(std::ostream&, int, const std::vector<std::__cxx11::basic_string<char> >&) const’:
/usr/include/ftk/filters/critical_point.hh:31:22: error: ‘critical_point_type_to_string’ was not declared in this scope
   31 |     os << "type=" << critical_point_type_to_string(cpdims, type, scalar_components.size()) << ", ";
      |                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /usr/include/ftk/filters/critical_point_tracker_regular.hh:6,
                 from ../src/plugins/metrics/ftk_critical_points.cc:5:
/usr/include/ftk/filters/critical_point_tracker.hh: In member function ‘void ftk::critical_point_tracker::write_traced_critical_points_text(std::ostream&) const’:
/usr/include/ftk/filters/critical_point_tracker.hh:406:33: error: ‘critical_point_type_to_string’ was not declared in this scope
  406 |     os << "consistent_type=" << critical_point_type_to_string(cpdims(), curve.consistent_type, scalar_components.size()) << ", ";
      |                                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/include/ftk/filters/critical_point_tracker.hh: In member function ‘void ftk::critical_point_tracker::grow_trajectories(std::vector<ftk::critical_point_traj_t>&, std::map<I, ftk::critical_point_t>&, std::function<std::set<T>(I)>, std::function<I(long long unsigned int)>)’:
/usr/include/ftk/filters/critical_point_tracker.hh:594:31: error: ‘extract_connected_components’ was not declared in this scope; did you mean ‘connected_components’?
  594 |   auto connected_components = extract_connected_components<I, std::set<I>>(
      |                               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |                               connected_components
/usr/include/ftk/filters/critical_point_tracker.hh:594:61: error: expected primary-expression before ‘,’ token
  594 |   auto connected_components = extract_connected_components<I, std::set<I>>(
      |                                                             ^
/usr/include/ftk/filters/critical_point_tracker.hh:599:31: error: ‘connected_component_to_linear_components’ is not a member of ‘ftk’
  599 |     auto linear_graphs = ftk::connected_component_to_linear_components<I>(component, neighbors);
      |                               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/include/ftk/filters/critical_point_tracker.hh:599:73: error: expected primary-expression before ‘>’ token
  599 |     auto linear_graphs = ftk::connected_component_to_linear_components<I>(component, neighbors);
      |                                                                         ^
/usr/include/ftk/filters/critical_point_tracker.hh: In member function ‘std::vector<ftk::critical_point_traj_t> ftk::critical_point_tracker::trace_critical_points_offline(const std::map<I, ftk::critical_point_t>&, std::function<std::set<T>(I)>)’:
/usr/include/ftk/filters/critical_point_tracker.hh:650:31: error: ‘extract_connected_components’ was not declared in this scope; did you mean ‘connected_components’?
  650 |   auto connected_components = extract_connected_components<element_t, std::set<element_t>>(
      |                               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |                               connected_components
/usr/include/ftk/filters/critical_point_tracker.hh:650:69: error: expected primary-expression before ‘,’ token
  650 |   auto connected_components = extract_connected_components<element_t, std::set<element_t>>(
      |                                                                     ^
/usr/include/ftk/filters/critical_point_tracker.hh:655:31: error: ‘connected_component_to_linear_components’ is not a member of ‘ftk’
  655 |     auto linear_graphs = ftk::connected_component_to_linear_components<element_t>(component, neighbors);
      |                               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/include/ftk/filters/critical_point_tracker.hh:655:81: error: expected primary-expression before ‘>’ token
  655 |     auto linear_graphs = ftk::connected_component_to_linear_components<element_t>(component, neighbors);
      |                                                                                 ^
ninja: build stopped: subcommand failed.
[runderwood@voyager libpressio]$

I am using gcc-10.1.0, cmake 3.18.1, libpressio 0.42.2, and ftk commit 556b0b09b8da064b84a237025ada6e31e8f050a7 (master) 8b9d72d460535856a45e9b7fb82048268cb3dcd6 (dev).

hguo commented 4 years ago

OK, looks like I removed a couple of headers accidentally. I'll add an integration test in FTK to prevent this to happen again. You should be able to compile with the latest version in the master branch now. Thanks.

robertu94 commented 4 years ago

@hguo Thanks for fixing that. It compiles now, but now I have a few more problems.

  1. I am currently getting a segfault in an ftk's calculation of the gradient: usr/include/ftk/ndarray/grad.hh:142 I suspect this is because i am not setting up the lattice or global dimensions properly, but I am unsure how to confirm if this is the case.

  2. I tried do run the same calculation using ./track_critcal_points like so:

./build/bin/track_critical_points -i ~/git/datasets/hurricane/100x500x500/CLOUDf48.bin.f64 --format float64 --dim 3 -w 500 -h 500 -d 100 -n 1 --output /tmp/foo --output-format text

In this example, CLOUDf48.bin.f64 is the file CLOUDf48.bin.f32 from SDRBench converted into double precision with the following python, I preform this conversion, because I preform an analogous conversion in the libpressio plugin.

import numpy as np
d = np.fromfile("CLOUDf48.bin.f32 ", dtype=np.float32)
d = np.reshape(500,500,100)
d2 = d.astype(np.float64)
d2.tofile("CLOUDf48.bin.f64")

However, I when I run ./track_critical_points get the following exception: 

terminate called after throwing an instance of 'cxxopts::option_not_exists_exception'
  what():  Option ‘format’ does not exist

Thread 1 "track_critical_" received signal SIGABRT, Aborted.
__GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
50        return ret;
(gdb)

Not being familiar with your command line parsing library, I hoped you could tell me what I am doing wrong.

Lastly, you had asked me how to test the plugin. If you use libpressio-tools that I shared with you a few weeks ago, you can run the following script to reproduce what I did:

pressio \
  -O all -M all -m time -m ftk_critical_points \
  -i ~/git/datasets/hurricane/100x500x500/CLOUDf48.bin.f32  -d 500 -d 500 -d 100 -t float \
  -o sz:error_bound_mode_str="abs" \
  -o sz:abs_err_bound=1e-3 \
  sz

to compile appropriate versions of it, you can use the following spack after adding my repository and configuring a passwordless ssh-key access for github:

spack install libpressio-tools@master ^libpressio@develop+ftk+mpi+libdistributed+sz
robertu94 commented 4 years ago

I've updated the ftk plugin based on your changes, and you can find the current draft on the develop branch of libpressio.

hguo commented 4 years ago

I’ll take a look. Btw, Can you confirm if CLOUDf48.bin.f64 is big endian or not? Thanks.

Sent from my iPhone

On Sep 1, 2020, at 12:14, Robert Underwood notifications@github.com wrote:

 I've updated the ftk plugin based on your changes, and you can find the current draft on the develop branch of libpressio.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

robertu94 commented 4 years ago

@hguo it is the version from /lcrc/project/ECP-EZ/public/compression/Hurricane-ISABEL/cleaned-data/CLOUDf48.bin.f32 with the transformation that I describe above. It has already been converted to little-endian (x86_64 native format) by Sheng prior to me downloading my copy.

hguo commented 4 years ago

Hi,

I have fixed the command line parsing problem and you should be able to use it as follows:

$ ./bin/track_critical_points -i isabel-sdrbench/CLOUDf48.bin.f32 -f float32 -w 500 -h 500 -d 100 --output isabel1.txt

I compiled and installed SZ, libdistributed, libpressio on my MBP, but and got the following error when compiling pressio-tools:

[ 11%] Linking CXX executable pressio Undefined symbols for architecture x86_64: "distributed::comm::serializer::serializer::bcast(pressio_data&, int, int)", referenced from: _main in pressio.cc.o

I nm'ed liblibdistributed.dylib and indeed did not find the symbol for this bcast function.

I tried the spack command you provided on both my MBP and an MCS machine, but got this error message: Error: Package 'libpressio_tools' not found. I have the latest develop branch of spack in both environments.

robertu94 commented 4 years ago

@hguo thanks for your reply. I'll take a look at the executable to see if I can figure out what I am doing differently.

Hi,

I have fixed the command line parsing problem and you should be able to use it as follows:

$ ./bin/track_critical_points -i isabel-sdrbench/CLOUDf48.bin.f32 -f float32 -w 500 -h 500 -d 100 --output isabel1.txt

I compiled and installed SZ, libdistributed, libpressio on my MBP, but and got the following error when compiling pressio-tools:

[ 11%] Linking CXX executable pressio Undefined symbols for architecture x86_64: "distributed::comm::serializer::serializer::bcast(pressio_data&, int, int)", referenced from: _main in pressio.cc.o

I nm'ed liblibdistributed.dylib and indeed did not find the symbol for this bcast function.

This symbol is provided by libpressio when it is compiled +mpi+libdistributed I assume you are compiling against mpich. I can try building libpressio against mpich to see if I get this error if that is what you are doing.

Alternatively you can pass -DLIBPRESSIO_HAS_MPI=ON -DLIBPRESSIO_HAS_LIBDISTRIBUTED=ON when configuring cmake.

I tried the spack command you provided on both my MBP and an MCS machine, but got this error message: Error: Package 'libpressio_tools' not found. I have the latest develop branch of spack in both environments.

The package is called libpressio-tools not libpressio_tools.

You may also need to add my spack repository

robertu94 commented 4 years ago

@hguo I found an error in the libpressio-tools spack where a minimal build would exhibit this linker error. It should be fixed now.

hguo commented 4 years ago

@robertu94 I tried "-DLIBPRESSIO_HAS_MPI=ON -DLIBPRESSIO_HAS_LIBDISTRIBUTED=ON" when configuring libpressio, but got the following comping error:

[ 1%] Building CXX object CMakeFiles/libpressio.dir/src/serializable.cc.o In file included from /Users/hguo/workspace/projects/libpressio/src/serializable.cc:1: In file included from /Users/hguo/workspace/projects/libpressio/include/libpressio_ext/cpp/serializable.h:8: /Users/hguo/workspace/projects/libdistributed/build/install/include/libdistributed/libdistributed_comm.h:732:58: error: no member named 'dtype' in 'distributed::comm::serializer::serializer' return MPI_Send(t.data(), t.size(), serializer::dtype(), dest, tag, comm);


/Users/hguo/workspace/projects/libdistributed/build/install/include/libdistributed/libdistributed_comm.h:988:37: note: in instantiation of member function
      'distributed::comm::serializer::serializer<std::__1::vector<unsigned long, std::__1::allocator<unsigned long> > >::send' requested here
  return serializer::serializer<T>::send(value, dest, tag, comm);
                                    ^
/Users/hguo/workspace/projects/libpressio/src/serializable.cc:48:18: note: in instantiation of function template specialization
      'distributed::comm::send<std::__1::vector<unsigned long, std::__1::allocator<unsigned long> > >' requested here
    ret |= comm::send(data.dimensions(), dest, tag, comm);
....

I am using MacOS w/ Apple clang, mpich.  I'll try your spack script later today on a Linux box.
robertu94 commented 4 years ago

@hguo I took your suggestion and added some CI for MacOS to libdistributed first (I'll add libpressio soon), but while libdistributed built, it didn't pass the unit tests, and I am unsure why and don't have a great way to debug that without Apple hardware. It passed all unit-tests with mpich and openmpi on Linux. For now, please only use libpressio and libdistributed from Linux.

robertu94 commented 3 years ago

@hguo FTK has grown a lot of new features since I last looked at it.

In the newest version of libpressio (0.57.0) I added support for returning more complex metrics via the external metrics module. This will allow users to write scripts to track the changes to FTK more closely as you release them and get the specific things they want out of it rather than me have to code a complex system to get specific things out in C. I think this is the long run better way to do this integration so I am closing this PR. I already have an example script that returns data from the ftk cli binary to libpressio that I can provide if needed.

hguo commented 3 years ago

@robertu94 Indeed, a lot has changed, and it does make sense to integrate things in a decoupled manner. Let me know where I could help in the future.