ianw-projects / toolkiticl Goto Github PK

We should expose data_rw_flags to the user, to be able to specify if some varibales are read/write only or both. This could be extended to support different memory copy options for multiple kernels in the future.

We could have an abstract base class and use virtual functions & inheritance for Intel, NVidia, AMD, .....

Up to now, the NVML device is hard coded in

We should add some command line option to choose another device or even other devices. A somewhat simple option would be to allow logging on only one device. However, I would prefer the ability to enable logging on an arbitrary number of devices.

As described at here, it would be better to use nvmlDeviceGetHandleByUUID or nvmlDeviceGetHandleByPciBusId.

CC @Kostaszki

On some Windows systems, there is an error error: failed to initialize MDH_Context if the prebuild binaries are used. Cf. #33 (comment).

Windows and Linux support is implemented in #18. For Mac OS X and Intel CPUs, we could either use MSR registers (as for Linux) or the Intel Power Gadget Library (as for Windows).

Based on the the discussion in PR #8, it might be a good idea to make the test build optional. Using something like '-DTESTS=ON' (as suggested by @ranocha ) with cmake would only include the tests, if explicitly specified by the user. This is also the way I'm used to from other projects. What do you think @Kostaszki and @ranocha?

It might be nice to add some data description (strings).

We should check possible errors while reading (and writing) HDF5 files and print appropriate error messages. For example, if settings/kernel_settings is missing, I get the cryptic output

Available devices: 1
Intel(R) Core(TM) i5-4590S CPU @ 3.00GHz
OpenCL version: OpenCL 2.1 (Build 0)
Memory limit: 8279801856
WG limit: 8192

Reading kernel from file: copy_kernel_py2.cl... 
HDF5-DIAG: Error detected in HDF5 (1.10.0-patch1) thread 140244994254656:
  #000: ../../../src/H5D.c line 296 in H5Dopen2(): unable to open dataset
    major: Dataset
    minor: Can't open object
  #001: ../../../src/H5Dint.c line 1463 in H5D__open_name(): not found
    major: Dataset
    minor: Object not found
  #002: ../../../src/H5Gloc.c line 430 in H5G_loc_find(): can't find object
    major: Symbol table
    minor: Object not found
  #003: ../../../src/H5Gtraverse.c line 869 in H5G_traverse(): internal path traversal failed
    major: Symbol table
    minor: Object not found
  #004: ../../../src/H5Gtraverse.c line 641 in H5G_traverse_real(): traversal operator failed
    major: Symbol table
    minor: Callback failed
  #005: ../../../src/H5Gloc.c line 385 in H5G_loc_find_cb(): object 'kernel_settings' doesn't exist
    major: Symbol table
    minor: Object not found
HDF5-DIAG: Error detected in HDF5 (1.10.0-patch1) thread 140244994254656:
  #000: ../../../src/H5D.c line 449 in H5Dget_type(): not a dataset
    major: Invalid arguments to routine
    minor: Inappropriate type
HDF5-DIAG: Error detected in HDF5 (1.10.0-patch1) thread 140244994254656:
  #000: ../../../src/H5T.c line 2045 in H5Tis_variable_str(): not a datatype
    major: Invalid arguments to routine
    minor: Inappropriate type
HDF5-DIAG: Error detected in HDF5 (1.10.0-patch1) thread 140244994254656:
  #000: ../../../src/H5D.c line 375 in H5Dget_space(): not a dataset
    major: Invalid arguments to routine
    minor: Inappropriate type
HDF5-DIAG: Error detected in HDF5 (1.10.0-patch1) thread 140244994254656:
  #000: ../../../src/H5S.c line 865 in H5Sget_simple_extent_ndims(): not a dataspace
    major: Invalid arguments to routine
    minor: Inappropriate type
terminate called after throwing an instance of 'std::bad_alloc'
  what():  std::bad_alloc

Cf. #14 (comment): We want to add a CMake switch to download cl2.hpp automatically.

Use HDF5 date and time format (see documentation) instead of custom strings.

This has been discussed in #18 (comment) and below. The basic question is:

So do we want to use human readable strings, or POSIX date format?

The strings are 24 character (incl. \0) ASCII strings, so they should have a size of 24 bytes each. The POSIX date format suggested in #18 (comment) uses Float64/double values, i.e. 8 bytes per date. They can be read and parsed faster by a computer but are not really human readable.

I can implement POSIX dates if we want to have them.

Based on the suggestion by @ranocha: Does it make sense so add the OpenCL platform name to the HDF5 output?

Does the platform name provide any more information compared to device name and OpenCL version? Or should we maybe implement a new structure, that joins all the necessary information in a single HDF5 struct or string?

• Temperature for Intel
• Temperature for AMD
• Output of sysfs
• Implement specific switch for AMD GPU power/temperature logging
• Fix concurrent power and temperature logging on AMD
• ARM?

Do we really want to set all the compiler flags of

Some reasons for the current choice are probably connected with clusters, where build machines have not the same architecture as the compute nodes. But this could be caught by some other variables passed to cmake. What is your opinion, @philipheinisch?

Right now, unknown command line arguments are simply ignored. This can be problematic and lead to confusion, e.g. if there is some typo.

Additionally, we might want to allow e.g. both -intel_temp and -intel_temperature.

Furthermore, we should print an error if the argument cannot be parsed correctly, e.g. the time in milliseconds for -nvidia_power.

It would be nice to extend the build system as discussed with @philipheinisch.

• Add TESTS option to cmake (for ccmake). Done in #30.
• Build all logging features and check availability at runtime.
• Allow prebuild binaries for Windows and some standard Linux (Ubuntu LTS?).
• Distibute prebuild binaries via github releases.
• Use some basic compiler optimisation flags, cf. #15. Done in #30.
  - [ ] Support CMAKE_BUILD_TYPE?
  The standard option seems to be something along the lineas of:

  set(CMAKE_CXX_FLAGS "-Wall -Wextra ...")
  set(CMAKE_CXX_FLAGS_DEBUG "-g")
  set(CMAKE_CXX_FLAGS_RELEASE "-O2 ...")
  

Currently, there are problems if the configuration file is not specified using a relative path in the same directory without path division signs /, \. For example,

$ toolkitICL -c ./repetition_test.h5 
Available devices: 1
Intel(R) Core(TM) i5-4590S CPU @ 3.00GHz
OpenCL version: OpenCL 2.1 (Build 0)
Memory limit: 8279801856
WG limit: 8192

Reading kernel from file: add_one_kernel.cl... 
Found Kernels: 1
Number of Kernels to execute: 5
Ingesting HDF5 config file...
Creating output HDF5 file...
HDF5-DIAG: Error detected in HDF5 (1.10.0-patch1) thread 140374442575680:
  #000: ../../../src/H5F.c line 491 in H5Fcreate(): unable to create file
    major: File accessibilty
    minor: Unable to open file
  #001: ../../../src/H5Fint.c line 1111 in H5F_open(): unable to open file: time = Fri Mar 15 08:26:03 2019
, name = 'out_./repetition_test.h5', tent_flags = 13
    major: File accessibilty
    minor: Unable to open file
  #002: ../../../src/H5FD.c line 812 in H5FD_open(): open failed
    major: Virtual File Layer
    minor: Unable to initialize object
  #003: ../../../src/H5FDsec2.c line 348 in H5FD_sec2_open(): unable to open file: name = 'out_./repetition_test.h5', errno = 2, error message = 'No such file or directory', flags = 13, o_flags = 242
    major: File accessibilty
    minor: Unable to open file
...

This came up in #11: Do we want to support OS X officially or at least as far as we can?

Pros:

• Many people like to use it.
• We can also run tests on OS X using Travis, possibly catching subtle bugs induced by the OS etc.

Cons/TODOs:

• I don't have access to a computer running OS x 😉 Hence, it's a bit difficult for me to get everything done correctly and test it.
• We would have to adapt many parts to an additional OS, e.g. getOS.
• We need to set up Travis for OS X. Right now, the main problem seems to be getting a working cl2.hpp
  

  toolkitICL/.travis.yml

  Line 40 in ae26c3c

  # osx image does not contain cl.hpp file; download from Khronos

  
  Edit: cf. https://rageandqq.github.io/blog/2018/03/09/opencl-mac-cpp.html

CC: @philipheinisch @Kostaszki