bluebrain / basalt Goto Github PK

For now, classes defined in src/basalt/circuit_payload depends on pybind11. It prevents unit-tests of these types because it is not possible to link against the pybind11 shared library.

pybind11 specific code should be defined elsewhere.

It is way too cumbersome to have this whenever someone wants to include a header.

https://github.com/tristan0x/basalt/blob/cd53d717021014b40eeef39f2f9e24830566d570/src/basalt/py_basalt.cpp#L10-L36

Update .travis.yaml to install clang-tidy

Add a new method to repush the vertex, useful when payload is modified.

Write a FindGoogleBenchmark.cmake providing:

• location to headers
• location to libraries
• library version

This script should be usable with FindPackage command, and follow CMake package file requirements.

Required by #22

Doxygen comments of header files in include/rocksdb should use the proper /** */ formatting as specified in the coding conventions

Basalt should use the last version available all its third-parties (in src/third_parties)

Use Google Benchmark library to add serialization tests.
Add C++ serialization with cereal and boost and compare with current naïve one.

Add begin() and end() member methods in basalt::connections_t like in basalt::nodes_t

Files layout of basalt repository, additional scripts, and development workflow should be as generic as possible to inspire how the C++/Python project template should be organized. See related issue BlueBrain/hpc-coding-conventions#32

Non-explicit list of changes:

• refactor dev/travis-build
• Rework Contributions section in README.md. Maybe create a dedicated CONTRIBUTING.md file...
• Move Python bindings in a dedicated directory i.e move src/basalt/py_* in a separate directory (src/python_bindings, src/basalt/bindings, ... ?)
• build wheels (see awesome work done by NSE)

Ensure that spack buids with flag -mnative. If not setup.py should set Basalt_ARCH CMake variable to native.

Document missing methods in the Skill graph example:

• VerticesWrapper get and __getitem__ methods
• Vertex wrapper class (id, type, data, serialize, deserialize)

Useful resource to tackle this task

source /gpfs/bbp.cscs.ch/project/proj62/NGV/workflow_modules.sh
source /gpfs/bbp.cscs.ch/project/proj62/NGV/workflow_venv/bin/activate
cd /gpfs/bbp.cscs.ch/project/proj62/NGV/TEST_ARCHNGV/h5_connectivity_data
python example.py

• iteration over nodes and connections. Pagination would be a plus to support parallelisation of readers.
• get number of nodes and connections.
• methods to test if a node or a connection exists

This issue focus on API, so implementation of these methods can remain empty in network.cc.

Use Jason Turner recommendations regarding naming conventions and compile with -Wshadow.

Ensure during CI that the CMake files installed by the make install target allow other CMake projects to use find_package(Basalt). So achieve such call, add a new separated CMake project that provides a program or a library that depends on basalt.

https://bbpteam.epfl.ch/repository/devpi/bbprelman/dev/sphinx-limestone-theme/0.0.4.dev0

CMake should raise a comprehensive error when submodules are missing.

Here is a FindCLANG_TIDY.cmake grabbed from CNS-OIST/STEPS

# /FindCLANG_TIDY.cmake
#
# This CMake script will search for clang-tidy and set the following
# variables
#
# CLANG_TIDY_FOUND : Whether or not clang-tidy is available on the target system
# CLANG_TIDY_EXECUTABLE : Fully qualified path to the clang-tidy executable
#
# The following variables will affect the operation of this script
# CLANG_TIDY_SEARCH_PATHS : List of directories to search for clang-tidy in,
#                           before searching any system paths. This should be
#                           the prefix to which clang-tidy was installed, and
#                           not the path that contains the clang-tidy binary.
#                           Eg /opt/ not /opt/bin/

# Search for the canonical executable, then search for ones with
# a version from newest to oldest.
find_program(CLANG_TIDY_EXECUTABLE
  NAMES clang-tidy clang-tidy-7
        clang-tidy-6.0 clang-tidy-5.0
        clang-tidy-4.0 clang-tidy-3.9 clang-tidy-3.8
        clang-tidy-3.7 clang-tidy-3.6 clang-tidy-3.5
  HINTS ${CLANG_TIDY_SEARCH_PATHS}
)

if(CLANG_TIDY_EXECUTABLE)
  set (CLANG_TIDY_FOUND TRUE)
else()
  set (CLANG_TIDY_FOUND FALSE)
endif()

Then CMake can call clang-tidy during every source code compilation:

option(USE_CLANG_TIDY "Perform C++ static analysis while compiling" FALSE)
if(USE_CLANG_TIDY)
    find_package(CLANG_TIDY REQUIRED)
    if(CLANG_TIDY_FOUND)
        cmake_minimum_required(VERSION 3.6)
        set(CLANG_TIDY_ARGS "" CACHE STRING "clang-tidy command options")
        set(CMAKE_CXX_CLANG_TIDY "${CLANG_TIDY_EXECUTABLE}" ${CLANG_TIDY_ARGS})
    else()
        message(ERROR "Could not find clang-tidy utility")
    endif()
endif()

Create C++ benchmark of reading operations as described in https://github.com/tristan0x/basalt/wiki/Use-Cases#use-cases to get baseline numbers.
Use Google Benchmark library to compute measurements.
Results should be added to /tests/benchmark directory.

If needed, bulk methods should be added to API to speed-up operations.

Development is made with Ubuntu package i.e version 4.1.1
It seems prefix queries do not work with RocksDB 5 (see faild build on OSX in TravisCI)

• import single-file header https://github.com/martinmoene/gsl-lite/
• replace gsl::owner by the one provided by gsl-lite
• use gsl::span in network_impl.hpp/cpp to fix clang-tidy cppcoreguildelines-pro-bounds-pointer-arithmetic warnings in network_impl.cpp

Use RocksDB as backend database

Related topic: https://www.alkaline-ml.com/2018-12-23-automate-gh-builds/

0.2.4 is 2MB, it is way too big. MANIFEST.in should be more restrictive, like for instance

global-exclude *.py[co] .DS_Store __pycache__ .git

Purpose is to track memory issues (access, leaks) during execution of unit-tests.
One build with gcc 7 / valgrind should be added to the TravisCI matrix.
Similar to valgrind support in CNS-OIST/STEPS repository.

https://github.com/eleftherioszisis/ArchNGV/blob/master/archngv/core/exporters/export_ngv_data.py
https://github.com/eleftherioszisis/ArchNGV/blob/master/scripts/mock_datasets/create_test_datasets.py

currently some methods return status_t, some return pairs of result and status. We should remove these inconsistencies and provide a unified interface with only status returned and results "returned" through reference arguments.

There is a CMake error when executing C++ unit-tests with valgrind

https://travis-ci.com/tristan0x/basalt/jobs/189494637

/usr/local/cmake-3.12.4/bin/cmake -E cmake_progress_start /home/travis/build/tristan0x/basalt/build/CMakeFiles 0
UpdateCTestConfiguration  from :/home/travis/build/tristan0x/basalt/build/DartConfiguration.tcl
Cannot find file: /home/travis/build/tristan0x/basalt/build/DartConfiguration.tcl
   Site: 
   Build name: (empty)
Create new tag: 20190402-0124 - Experimental
UpdateCTestConfiguration  from :/home/travis/build/tristan0x/basalt/build/DartConfiguration.tcl
Cannot find file: /home/travis/build/tristan0x/basalt/build/DartConfiguration.tcl
Memory check project /home/travis/build/tristan0x/basalt/build
Memory checker (MemoryCheckCommand) not set, or cannot find the specified program.
Errors while running CTest

See https://bbpteam.epfl.ch/project/spaces/display/BBPNSE/Python

For instance:

• use doc/ instead of docs
• structure of documentation is different

  doc
  ├── Makefile
  └── source
      ├── conf.py
      └── index.rst
  

There are a couple of warnings raised by clang-tidy. We should fix the one that applies to basalt, and ignores those commit from third-parties.

Also update clang-tidy configuration according to Jason Turner recommandation

If possible make .clang-tidy configuration file generic (without hard-coded paths of include-directories to excludes for instance) so that it can be used later for every C++ project.

Waits for #32 to be merged as it provides small change regarding arguments given to clang-tidy by CMake.

Hard-coded version in CMakeLists.txt should be also removed.
bob/cmake should be updated so that it does not need Basalt_VERSION and compute it somehow like setuptools_scm is doing.

Of course other ideas are welcome

We should consider using this container because using stack allocated memory may be faster for small payload.
See: https://github.com/abseil/abseil-cpp/blob/master/absl/container/inlined_vector.h

Sentry is an amazing tool to track errors in every language, framework, and library. I used it to get real-time alerting in Slack of errors that occurred in production systems of SAAS services (Python Flask, Javascript, ...). I have just realised that Sentry is part of GitHub education program
If Sentry supports C++, that could be interesting to see if it can be used to be notified of errors while our softwares are being used.

Please fix/update this wiki page

When doing pip install /path/to/basalt-0.2.4.tar.gz:

  
    Configuration error:
    There is a programmable error in your configuration file:
    
    Traceback (most recent call last):
      File "/home/tcarel/src/github.com/pyenv/pyenv/versions/3.6.8/lib/python3.6/site-packages/sphinx/config.py", line 368, in eval_config_file
        execfile_(filename, namespace)
      File "/home/tcarel/src/github.com/pyenv/pyenv/versions/3.6.8/lib/python3.6/site-packages/sphinx/util/pycompat.py", line 150, in execfile_
        exec_(code, _globals)
      File "/tmp/pip-install-gz6cmxud/basalt/docs/conf.py", line 22, in <module>
        import basalt
      File "/tmp/pip-install-gz6cmxud/basalt/basalt/__init__.py", line 7, in <module>
        from ._basalt import Status, Vertices, Edges, Graph, make_id
    ModuleNotFoundError: No module named 'basalt._basalt'

Because the test command currently needs to be executed before.

Goals of this ticket is to:

• Fix the build command so that it properly copies the shared libraries in the basalt directory (like the test command is doing).
• add a Dockerfile in the dev directory to build a container that can test pip install of a distribution tarball in a minimalist environment (python3.5, setuptools, and pip).

basalt is currently compiled with many -Wno-... compilation flags to get around many warnings in spdlog library, especially with clang. (see cmake/bob.cmake)

Tasks list:

1. fork gabime/spdlog and use it instead as submodule
2. fix all warnings
3. submit a pull-request
4. use official spdlog when pull-request merged and released.

For unit-tests, it would be very convenient to have tiny versions of H5 connectivity data, let's say 3 astrocytes max.
You can add them in tests/py/data/ directory.

Build should pass with #include "rocksdb/db.h" added to src/basalt/network.cc

I would like to access the ngv_connectome and ngv_data from basalt using an interface similar to the ones in basalt/ngv. Following our discussion with @tristan0x we agreed to ignore feature based information retrieval for basalt and use hdf5 for that in the steps of the pipeline that information is required in bulk by feature. Information that is retrieved per astrocyte / synapse / neuron / Vasculature Segment is more suitable for basalt.

So far, I was first querying the NGVConnectome in order to retrieve the indices for the neighbours, and then extracted their respective data from NGVData. This has to change given the basalt organization where the data is stored in the nodes.

The goal is to be able to query basalt for an astrocyte id and retrieve its information and neighbors. For example, the information required for one astrocyte for the morphology synthesis is the following:

• soma_position, radius
• microdomain geometry (vertex_coordinates, triangles )
• neuroglial_connectivity's synapses assigned to astrocyte's microdomain
• gliovascular_connectivity's endfeet surface targets

I think it would make sense to first focus on the astrocyte perspective.

The process seems to not be in the proper directory

https://travis-ci.com/tristan0x/basalt/jobs/189494639

-- Build files have been written to: /Users/travis/build/tristan0x/basalt/dist/basalt-0.2.3/build/temp.macosx-10.13-x86_64-3.6
Running tests...
Test project /Users/travis/build/tristan0x/basalt/dist/basalt-0.2.3/build/temp.macosx-10.13-x86_64-3.6
    Start 1: unit-tests
1/1 Test #1: unit-tests .......................   Passed    0.12 sec
100% tests passed, 0 tests failed out of 1
Total Test time (real) =   0.13 sec
running doctest
Application error:
config directory doesn't contain a conf.py file (docs)
Traceback (most recent call last):
  File "setup.py", line 192, in <module>
    """,
  File "/usr/local/lib/python3.6/site-packages/setuptools/__init__.py", line 129, in setup
    return distutils.core.setup(**attrs)
  File "/usr/local/Cellar/python/3.6.5_1/Frameworks/Python.framework/Versions/3.6/lib/python3.6/distutils/core.py", line 148, in setup
    dist.run_commands()
  File "/usr/local/Cellar/python/3.6.5_1/Frameworks/Python.framework/Versions/3.6/lib/python3.6/distutils/dist.py", line 955, in run_commands
    self.run_command(cmd)
  File "/usr/local/Cellar/python/3.6.5_1/Frameworks/Python.framework/Versions/3.6/lib/python3.6/distutils/dist.py", line 974, in run_command
    cmd_obj.run()
  File "setup.py", line 137, in run
    subprocess.check_call([sys.executable, __file__, "doctest"])
  File "/usr/local/Cellar/python/3.6.5_1/Frameworks/Python.framework/Versions/3.6/lib/python3.6/subprocess.py", line 291, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['/usr/local/opt/python/bin/python3.6', 'setup.py', 'doctest']' returned non-zero exit status 1.

For now, this step has been disabled in setup.py on MacOS in cbbe609