Omp #38

Ivanmartinezperez · 2016-10-26T08:22:58Z

1-OpenMP back end working without tasks (-DWITH_OMP).
2-Extrae+Paraver support added, now just working on Marenostrum due to the path specified(-DWITH_EXTRAE).

merge from main

Also: * Ensure intrinsic and passive properties properly set on test cells.

* Align defaults with values used in most of the NEURON validation scripts. * Use consistent 100 Ω·m bulk resistivity across both NEURON test models and basic validation cells.

* Move generation and data to top-level validation directory. * Make BUILD_VALIDATION_DATA and VALIDATION_DATA_DIR cache vars. * Add helper CMake functions for data generation. Note `validation/ref/numeric/foo.sh` is just a placeholder.

* Use consistent scaling for y[1] scalar voltage in hh_soma.jl * Also: add more reserved target names to CMakeLists.txt helper function.

* Amend data directory path in validation tests. * Enmodulate `hh_soma.jl` * Add HH channel reference data generations script. * Switch `validate_soma.cpp` to numeric reference data. * Consolidate common code in `validate_ball_and_stick.cpp` * Add (nearly) Rallpack1 validation test (see below). * Gentle failure on absence of reference data in `validate_ball_and_stick.cpp` Can't yet override mechanism default parameter values, so the cable cell model added to `test_common_cells.hpp` lets the default stand; validation script will have to use the default membrane conductance rather than that given by Rallpack1.

* Implement Rallpack1 validation test (with a workaround for inability to set membrane conductance). * Fix bug in L≠1 case in PassiveCable.jl (this may still be wrong). * Fix bug in peak delta computation in trace analysis when both traces have no local maxima. * Gentle failure on missing `numeric_soma.json` * Allow multiple `-s` selection operations for `tsplot`, acting disjunctively.

bcumming

There are some small fixes to make in the code.

The bigger changes are in how the extrae version is compiled. System-specific information in the CMakeLists.txt, especially hard-coded paths, are really brittle, and will become a real mess as we work on more systems. I have made some comments that suggest fixes for this.

bcumming · 2016-10-26T08:31:55Z

CMakeLists.txt

@@ -39,6 +39,16 @@ if(WITH_TBB)
    add_definitions(-DWITH_TBB)
 endif()

+# OpenMP support
+set(WITH_OMP OFF CACHE BOOL "use OpenMP for on-node threading" )


With OpenMP we have 3 threading back ends (OpenMP, TBB, serial). We can't use two at the same time (i.e. it shouldn't be possible to have WITH_OMP and WITH_TBB true.

How about a single variable, named something like THREADING_MODEL, that the user sets to "tbb", "openmp" or "serial". You can test this variable, and set WITH_OMP, WITH_TBB appropriately.

If the user specifies something different (i.e. THREADING_MODEL=foo) We load serial or just throw and error?

PD: If THREADING_MODEL is not specified, serial is the default model

I think that it is reasonable to default to serial, and then throw an error if the user provides an unsupported/unknown back end. Silently just using serial if an invalid back end is requested will cause confusion.

bcumming · 2016-10-26T08:33:40Z

src/model.hpp

@@ -182,7 +182,7 @@ class model {
            // run the tasks, overlapping if the threading model and number of
            // available threads permits it.
            threading::task_group g;
-            g.run(exchange);
+            g.run(exchange);            


white space at end of line.

bcumming · 2016-10-26T08:34:43Z

src/threading/omp.hpp

+#pragma once
+
+#if !defined(WITH_OMP)
+    #error "this header can only be loaded if WITH_SERIAL is set"


should be

"this header can only be loaded if WITH_OMP is set"

bcumming · 2016-10-26T08:35:56Z

src/threading/omp.hpp

+template <typename RandomIt>
+void sort(RandomIt begin, RandomIt end) {
+    pss::parallel_stable_sort(begin, end);
+    //std::sort(begin, end);


remove commented out lines (see the one below too)

bcumming · 2016-10-26T08:36:22Z

src/threading/omp.hpp

+    //std::sort(c.begin(), c.end());
+}
+
+// FIXME


Can these comments be removed?

bcumming · 2016-10-26T08:38:01Z

src/threading/omp.hpp

+
+constexpr bool multithreaded() { return true; }
+
+/// Proxy for tbb task group.


These comments need to be updated for the OpenMP version.

bcumming · 2016-10-26T09:04:55Z

run-once.sh

@@ -0,0 +1,15 @@
+#!/bin/bash


The run-once.sh and run_instrumented.sh scripts

they don't belong in the root path.

have system specific information (i.e. Mare Nostrum).

the naming, with underscore and hyphen, is inconsistent.

I would make a new path scripts/system-marenostrum and put them in there (along with extrae.xml)

bcumming · 2016-10-26T09:16:22Z

cmake/CompilerOptions.cmake

@@ -4,6 +4,8 @@ set(CXXOPT_DEBUG "-g")
 set(CXXOPT_PTHREAD "-pthread")
 set(CXXOPT_CXX11 "-std=c++11")
 set(CXXOPT_WALL "-Wall")
+set(BSCOPT_EXTRAE "-O -g -I $(EXTRAE_HOME)/include")


These options have three bits of information
1. debug flags -g
2. optimization flags -O
3. an include path

When WITH_EXTRAE is set in the main CMakeLists, you should check if -g has been added to CMAKE_CXX_FLAGS, and add it if needed.

You should not specify the optimization flag, which might conflict with the default set by the system.

Set the include path using the CMake include_directories(${EXTRAE_INCLUDE}) in the main CMakeLists.txt. Make the user provide EXTRAE_INCLUDE (maybe an EXTRAE_ROOT like for TBB)

bcumming · 2016-10-26T09:18:44Z

cmake/CompilerOptions.cmake

@@ -4,6 +4,8 @@ set(CXXOPT_DEBUG "-g")
 set(CXXOPT_PTHREAD "-pthread")
 set(CXXOPT_CXX11 "-std=c++11")
 set(CXXOPT_WALL "-Wall")
+set(BSCOPT_EXTRAE "-O -g -I $(EXTRAE_HOME)/include")
+set(BSCLINK_EXTRAE "-Wl,-rpath -Wl,/apps/CEPBATOOLS/extrae/latest/impi5_mt+libgomp4.9/64/lib -L/apps/CEPBATOOLS/extrae/latest/impi5_mt+libgomp4.9/64/lib")


System specific information like this is very brittle. I would make the user provide an EXTRAE_LINK with this information. You could include a READMD.md in the scripts/system-marenostrum path (see my comment below), that explains how to build.

The way to do this properly would be to write a CMake module, something like extrae.find, to automate this process.

Feature/cable validation

* Fix quoting error in library search. * Add "lib" to prefixes when system is "Linux".

Julia 0.5 deprecates use of `symbol` instead of `Symbol`. This patch just substitutes the correct call.

…ning Address deprecated use of 'symbol' warning.

Add "lib" to search prefixes for libtbb

…units modcc: Support for units syntax within a state block.

* Tests for `math::pi`, `math::lerp`, `math::area_frustrum` and `math::volume_frustrum` * Fix `math:pi<long double>()`.

* New `filter` view: lazily selects based on predicate. * Generic `front` and `back` for sequences. * New rangeutil STL wrappers `stable_sort_by`, `all_of`, `any_of`. * Consolidate common utility unit testing structures into `tests/unit/common.hpp`

* Make `test_common_cells.hpp` and `ball_and_taper.py` agree. * Add `ball_and_squiggle` model that has a tapering undulating profile.

* Add documentation of template parameters for `filter_iterator`. * Document use of `uninitalized<F>` for holding functional objects in `filter_iterator` and `transform_iterator`

More range functionality, unit tests.

Feature/new test model

* Simplify trace analysis and reporting code in `trace_analysis.hpp` * Consolidate convergence test run procedures into new class `convergence_test_runner`.

* Make `algorithm::sum`, `algorithm::mean` more generic, allowing use with array types. * Add `div_compartment` compartment representation, that holds geometric information for each half of a compartment that will then be used in calculating control volumes. * Add three compartmentalisation schemes/policies that discretize a segment into `div_compartment` objects: * `div_compartment_by_ends` divides based only on the segment end points and radii. * `div_compartment_sampler` forms frusta by sampling the segment radius at each compartment boundary * `div_compartment_integrator` computes the compartment areas and volumes exactly by summing all frustra in the intersection of the segment and the compartmnet span.

…idation-tests Consolidate validation tests (issue arbor-sim#41)

Ivanmartinezperez · 2016-10-28T11:46:46Z

1- We need to modify the README.md, right now we use -DTHREADING_MODEL=omp or -DTHREADING_MODEL=tbb to select the threading model.

2-Extrae linked through an script using LD_PRELOAD environment variable, if you want to extract a trace, just execute the scripts inside Extrae folder.

3-I need to write a readme inside the Extrae folder!

…ments Feature/divided compartments

* Use divided compartments to determine FVM coefficients. * Pick correct control volume in FVM from sgement position (avoids off-by-half error.) * Add colour override functionality to tsplot: `--colour` option. * Add const accessor for cell soma. * Source formatting, comments in `math.hpp` * Fix `range_view`: was using incorrectly named type trait. * Add unit test for `range_view`. * Allow points of discontinuity to be omitted from L-infinity norm calculations. * Add `-d, --min-dt` option to `validate.exe` to control time step in validation convergence tests. * Add validation test: confirm divided compartment policy does not effect results on simple frustrum dendrites. * Change default max compartments on validation tests to 100 (ad hoc observed convergence limit at dt circa 0.001 ms; finder spatial division would required much finer dt.) * Make NEURON validation data generation scripts use CVODE by default, and with `secondorder=2` when non-zero `dt` is given.

* Use only `div_compartment_integrator` for compartmentalization in `fvm_multicell`. The policy will later be moved to a backend policy class. * For now, disable validation tests that test different division policies (see above). * Tweak comments and remove redundant `using`, following comments on PR#54.

…ments Complex compartments

bcumming · 2016-10-31T08:24:31Z

CMakeLists.txt

-# TBB support
-set(WITH_TBB OFF CACHE BOOL "use TBB for on-node threading" )
-if(WITH_TBB)
+#threading model selection


This is better. But I think that we should make THREADING_MODEL=serial the default, and throw an error if the user requests an invalid model.

set(THREADING_MODEL "serial" CACHE STRING "set the threading model, one of serial/tbb/omp")

bcumming · 2016-10-31T08:27:07Z

scripts/Extrae/run_instrumented_omp.sh

@@ -0,0 +1,30 @@
+#!/bin/bash


Looks good! We have a policy (that isn't written anywhere... sorry), of not using upper case in path names, so could you please rename the path to scripts/extrae,

bcumming · 2016-11-02T08:47:32Z

CMakeLists.txt

+#threading model selection
+if(THREADING_MODEL MATCHES "tbb")
+    # TBB support
+    set(WITH_TBB OFF CACHE BOOL "use TBB for on-node threading" )


the WITH_TBB and WITH_OMP variables below are no longer need if we use THREADING_MODEL to describe the threading backend.

Fixes validation build failure on Juwels. * More involved wrapping of CMake FindPkgConfig module for NetCDF discovery, including the use of `LIBRARY_PATH` which apparently CMake does not bother to look in by default when performing `find_library()` searches. * Add netcdf module load to juwels-mc environment script.

bcumming and others added 15 commits October 13, 2016 15:58

Merge pull request arbor-sim#1 from eth-cscs/master

039d5a9

merge from main

first version of openmp threading back end

7e0a089

Merge branch 'omp' of github.com:bcumming/nestmc-proto into omp

31beaca

adding openmp parallel sort implementation

f96499a

OpenMP sort working

e32842c

Support for units syntax within state block.

b6aec69

Add soma-less cable cell to test cells.

e751fcd

Also: * Ensure intrinsic and passive properties properly set on test cells.

Change bulk resistivity default.

d2ce7a6

* Align defaults with values used in most of the NEURON validation scripts. * Use consistent 100 Ω·m bulk resistivity across both NEURON test models and basic validation cells.

OpenMP back end working

7047383

Add Extrae+paraver support, needs to fix compilation warnings

029a3da

Reorganize validation data generation

b263eb2

* Move generation and data to top-level validation directory. * Make BUILD_VALIDATION_DATA and VALIDATION_DATA_DIR cache vars. * Add helper CMake functions for data generation. Note `validation/ref/numeric/foo.sh` is just a placeholder.

Bugfix: hh_soma.jl

7845818

* Use consistent scaling for y[1] scalar voltage in hh_soma.jl * Also: add more reserved target names to CMakeLists.txt helper function.

Remove errant test file.

1579e2c

bcumming requested changes Oct 26, 2016

View reviewed changes

Ivanmartinezperez and others added 14 commits October 26, 2016 13:18

file's cleanup

f572e62

Remove tabs

8dbdde3

Use correct routine in numeric_rallpack1.jl x0.3

07d7e2b

Configure-time test for julia

ddc8537

math::infinity<>() wrapper for infinity

104e389

Use name i_e for Stim current density

84ec4f5

Use math::infinity<>() for infinite value

1a9db4a

Merge pull request arbor-sim#37 from halfflat/feature/cable-validation

49ed243

Feature/cable validation

Merge branch 'master' into bugfix/modcc/state-block-units

18a9226

Adds unit tests for the STATE block.

05e4770

Add "lib" to search prefixes for libtbb

3201d72

* Fix quoting error in library search. * Add "lib" to prefixes when system is "Linux".

Address deprecated use of 'symbol' warning.

dbca304

Julia 0.5 deprecates use of `symbol` instead of `Symbol`. This patch just substitutes the correct call.

Address deprecated use of 'symbol' warning.

78c8e12

Julia 0.5 deprecates use of `symbol` instead of `Symbol`. This patch just substitutes the correct call.

Merge pull request arbor-sim#44 from halfflat/bugfix/julia-symbol-war…

0a37a6d

…ning Address deprecated use of 'symbol' warning.

bcumming and others added 15 commits October 27, 2016 13:56

Merge pull request arbor-sim#39 from halfflat/bugfix/findtbb-lib-search

0e4970d

Add "lib" to search prefixes for libtbb

Addresses PR comments.

9e0b874

Merge pull request arbor-sim#35 from vkarak/bugfix/modcc/state-block-…

f1eac25

…units modcc: Support for units syntax within a state block.

Merge remote-tracking branch 'upstream/master'

5174b69

Unit tests for math.hpp

a39c9a3

* Tests for `math::pi`, `math::lerp`, `math::area_frustrum` and `math::volume_frustrum` * Fix `math:pi<long double>()`.

Extend range, view functionality.

63c507b

* New `filter` view: lazily selects based on predicate. * Generic `front` and `back` for sequences. * New rangeutil STL wrappers `stable_sort_by`, `all_of`, `any_of`. * Consolidate common utility unit testing structures into `tests/unit/common.hpp`

Add ball_and_squiggle model; fix ball_and_taper.

830428a

* Make `test_common_cells.hpp` and `ball_and_taper.py` agree. * Add `ball_and_squiggle` model that has a tapering undulating profile.

Address PR#46 review comments.

cee495c

* Add documentation of template parameters for `filter_iterator`. * Document use of `uninitalized<F>` for holding functional objects in `filter_iterator` and `transform_iterator`

Merge pull request arbor-sim#46 from halfflat/feature/more-range-utils

c97135d

More range functionality, unit tests.

Merge pull request arbor-sim#47 from halfflat/feature/new-test-model

5ade8d0

Feature/new test model

Consolidate validation test code (issue arbor-sim#41)

1b929ff

* Simplify trace analysis and reporting code in `trace_analysis.hpp` * Consolidate convergence test run procedures into new class `convergence_test_runner`.

Merge pull request arbor-sim#48 from halfflat/feature/consolidate-val…

550da10

…idation-tests Consolidate validation tests (issue arbor-sim#41)

Extrae linked at execution time

5fc61ae

cleaning project

22b7391

bcumming and others added 4 commits October 28, 2016 15:01

Merge pull request arbor-sim#49 from halfflat/feature/divided-compart…

a03af27

…ments Feature/divided compartments

Merge pull request arbor-sim#54 from halfflat/feature/complex-compart…

e8d3285

…ments Complex compartments

bcumming reviewed Nov 2, 2016

View reviewed changes

Ivanmartinezperez and others added 2 commits November 2, 2016 13:00

Minor twicks and corrections

d2b7810

Solving conflicts

d4c6992

bcumming approved these changes Nov 4, 2016

View reviewed changes

bcumming merged commit 0ded25a into arbor-sim:master Nov 4, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Omp #38

Omp #38

Ivanmartinezperez commented Oct 26, 2016

bcumming left a comment

bcumming Oct 26, 2016 •

edited

Loading

Ivanmartinezperez Oct 26, 2016

bcumming Oct 26, 2016

bcumming Oct 26, 2016

bcumming Oct 26, 2016

bcumming Oct 26, 2016

bcumming Oct 26, 2016

bcumming Oct 26, 2016

bcumming Oct 26, 2016 •

edited

Loading

bcumming Oct 26, 2016

bcumming Oct 26, 2016

bcumming Oct 27, 2016

Ivanmartinezperez commented Oct 28, 2016

bcumming Oct 31, 2016

bcumming Nov 2, 2016

bcumming Oct 31, 2016

bcumming Nov 2, 2016


		constexpr bool multithreaded() { return true; }

		/// Proxy for tbb task group.

Omp #38

Omp #38

Conversation

Ivanmartinezperez commented Oct 26, 2016

bcumming left a comment

Choose a reason for hiding this comment

bcumming Oct 26, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bcumming Oct 26, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ivanmartinezperez commented Oct 28, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bcumming Oct 26, 2016 •

edited

Loading

bcumming Oct 26, 2016 •

edited

Loading