Skip to content

Commit

Permalink
Merge branch 'master' into attempted-defenestration-of-nmodl-home
Browse files Browse the repository at this point in the history
  • Loading branch information
matz-e authored Jul 22, 2024
2 parents c765a3a + b91c0ea commit d97d88c
Show file tree
Hide file tree
Showing 52 changed files with 620 additions and 194 deletions.
45 changes: 37 additions & 8 deletions .github/workflows/nmodl-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,14 @@ jobs:
os: ubuntu-20.04
- config:
flag_warnings: ON
glibc_asserts: ON
os: ubuntu-22.04
- config:
os: macos-12
- config:
os: macos-13
- config:
os: windows-latest
# TODO: might be interesting to add the thread sanitizer too
- config:
os: ubuntu-22.04
Expand Down Expand Up @@ -82,6 +85,15 @@ jobs:
echo CMAKE_BUILD_PARALLEL_LEVEL=2 >> $GITHUB_ENV
shell: bash

- name: Install choco packages
if: startsWith(matrix.config.os, 'windows')
run: |
choco install winflexbison3
- name: Configure VS Toolchain (Windows)
if: startsWith(matrix.config.os, 'windows')
uses: ilammy/msvc-dev-cmd@v1

- name: Set up Python3
uses: actions/setup-python@v5
with:
Expand All @@ -108,12 +120,17 @@ jobs:
if: ${{matrix.config.sanitizer}}
run: echo "::add-matcher::.github/problem-matchers/${{matrix.config.sanitizer}}.json"

- name: Configure
- name: Configure Windows
if: startsWith(matrix.config.os, 'windows')
run: |
cmake -S . -B build -DFLEX_INCLUDE_PATH=C:/ProgramData/chocolatey/lib/winflexbison3/tools -DBUILD_SHARED_LIBS=OFF
- name: Configure Linux/MacOS
if: ${{ !startsWith(matrix.config.os, 'windows') }}
shell: bash
working-directory: ${{runner.workspace}}/nmodl
run: |
echo "------- Configure -------"
mkdir build && pushd build
cmake_args=(-G Ninja
-DPYTHON_EXECUTABLE=$(which python3) \
-DCMAKE_INSTALL_PREFIX=$INSTALL_DIR)
Expand All @@ -134,15 +151,20 @@ jobs:
CXX=$(command -v clang++-14)
else
CXX=${CXX:-g++}
if [[ -n "${{matrix.config.glibc_asserts}}" ]]; then
cmake_args+=(-DCMAKE_BUILD_TYPE=Debug \
-DCMAKE_CXX_FLAGS=-D_GLIBCXX_DEBUG)
fi
fi
cmake_args+=(-DCMAKE_CXX_COMPILER=${CXX} \
-DCMAKE_CXX_COMPILER_LAUNCHER=ccache)
cmake_args+=(-DNMODL_ENABLE_BACKWARD=On)
cmake .. "${cmake_args[@]}"
cmake -B build -S . "${cmake_args[@]}"
env:
INSTALL_DIR: ${{ runner.workspace }}/install

- name: Dump config dictionary
if: ${{ !startsWith(matrix.config.os, 'windows') }}
run: |
cat << EOF > matrix.json
${{toJSON(matrix.config)}}
Expand All @@ -152,6 +174,7 @@ jobs:
echo -----
- name: Restore compiler cache
if: ${{ !startsWith(matrix.config.os, 'windows') }}
uses: actions/cache@v4
with:
path: |
Expand All @@ -162,9 +185,15 @@ jobs:
${{hashfiles('matrix.json')}}-
save-always: true

- name: Build
- name: Build Windows
if: startsWith(matrix.config.os, 'windows')
run: |
cmake --build build
- name: Build Linux/MacOS
if: ${{ !startsWith(matrix.config.os, 'windows') }}
shell: bash
working-directory: ${{runner.workspace}}/nmodl/build
working-directory: ${{runner.workspace}}/nmodl
env:
CCACHE_BASEDIR: ${{runner.workspace}}/nmodl
CCACHE_DIR: ${{runner.workspace}}/ccache
Expand All @@ -180,7 +209,7 @@ jobs:
ccache -z
# Older versions don't support -v (verbose)
ccache -vs 2>/dev/null || ccache -s
cmake --build .
cmake --build build
ccache -vs 2>/dev/null || ccache -s
- name: Test
shell: bash
Expand All @@ -191,9 +220,9 @@ jobs:
ctest --output-on-failure -T Test
- name: Install
shell: bash
working-directory: ${{runner.workspace}}/nmodl/build
working-directory: ${{runner.workspace}}/nmodl
run: |
cmake --build . --target install
cmake --install build
- name: Update references
if: matrix.config.update_references == 'On'
Expand Down
3 changes: 3 additions & 0 deletions .gitmodules
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,6 @@
[submodule "ext/backward"]
path = ext/backward
url = https://github.com/bombela/backward-cpp.git
[submodule "ext/dlfcn-win32"]
path = ext/dlfcn-win32
url = https://github.com/dlfcn-win32/dlfcn-win32
5 changes: 5 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,11 @@ if(NOT NMODL_3RDPARTY_USE_FMT
endif()
cpp_cc_git_submodule(json BUILD PACKAGE nlohmann_json REQUIRED)
cpp_cc_git_submodule(pybind11 BUILD PACKAGE pybind11 REQUIRED)
if(WIN32)
cpp_cc_git_submodule(dlfcn-win32 BUILD)
add_library(dlfcn-win32::dl ALIAS dl)
set(CMAKE_DL_LIBS dlfcn-win32::dl)
endif()
# Tell spdlog not to use its bundled fmt, it should either use the fmt submodule or a truly external
# installation for consistency. This line should be harmless if we use an external spdlog.
option(SPDLOG_FMT_EXTERNAL "Force to use an external {{fmt}}" ON)
Expand Down
2 changes: 1 addition & 1 deletion azure-pipelines.yml
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ stages:
submodules: True
- script: |
brew install flex bison cmake python@3
python3 -m pip install --upgrade pip setuptools
python3 -m pip install --upgrade pip "setuptools<=70.3.0"
python3 -m pip install --user -r requirements.txt
displayName: 'Install Dependencies'
- script: |
Expand Down
6 changes: 4 additions & 2 deletions cmake/FlexHelper.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@
get_filename_component(FLEX_BIN_DIR ${FLEX_EXECUTABLE} DIRECTORY)

if(NOT FLEX_BIN_DIR MATCHES "/usr/bin")
get_filename_component(FLEX_INCLUDE_PATH ${FLEX_BIN_DIR} PATH)
set(FLEX_INCLUDE_PATH ${FLEX_INCLUDE_PATH}/include/)
if(NOT FLEX_INCLUDE_PATH)
get_filename_component(FLEX_INCLUDE_PATH ${FLEX_BIN_DIR} PATH)
set(FLEX_INCLUDE_PATH ${FLEX_INCLUDE_PATH}/include/)
endif()
if(EXISTS "${FLEX_INCLUDE_PATH}/FlexLexer.h")
message(STATUS " Adding Flex include path as : ${FLEX_INCLUDE_PATH}")
include_directories(${FLEX_INCLUDE_PATH})
Expand Down
41 changes: 41 additions & 0 deletions docs/contents/globals.rst
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ The following global variables exist:

* ``GLOBAL`` visible from HOC/Python.
* ``LOCAL`` at file scope, called top-locals.
* ``CONSTANT`` not visible from HOC/Python.

GLOBAL variables
================
Expand Down Expand Up @@ -175,6 +176,46 @@ Collection of slightly surprising behaviour:
the code ``nocmodl`` produces will cause a SEGFAULT.


PARAMETER variables
===================

These can be either RANGE or not RANGE. They can be both read and write. If
they're written to, they're converted to thread-variables. Therefore, the rest
of this section will only describe read-only PARAMETERs.

Additionally, parameters optionally have: a) a default value, b) units and c) a
valid range.

Default Values
~~~~~~~~~~~~~~
This section only applies to read-only PARAMETERs.

The behaviour differs for RANGE variables and non-RANGE variables. For RANGE
variables, default values need to be registered with NEURON for all PARAMETERs
that are RANGE variables. The function is called ``hoc_register_parm_default``.

Note, that NOCMODL uses it in the ``nrn_alloc`` in the generated `.cpp` files
and also in ``ndatclas.cpp``. Therefore, it seems registering the values isn't
optional.

Non-RANGE variables are semantically equivalent to ``static double``. They're
simply assigned their value in the definition of the global variable.

CONSTANT variables
==================

These are comparatively simple. In NOCMODL they're implemented as non-const
static doubles. They're not accessible from HOC/Python (which makes them
simple).

Quirks
~~~~~~

In certain versions of NOCMODL around `9.0` and before (and NMODL) it's
possible to change the value of CONSTANT variables. The MOD file will still be
considered "thread-safe" (even if it might not be).


What Does CoreNEURON support?
=============================
CoreNEURON only supports read-only GLOBAL variables. Anything else needs to be
Expand Down
1 change: 1 addition & 0 deletions ext/dlfcn-win32
Submodule dlfcn-win32 added at 048bff
3 changes: 3 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -74,3 +74,6 @@ environment = { PATH = "/nmodlwheel/flex/bin:/nmodlwheel/bison/bin:$PATH" }
# the Linux wheel is not tested in cibuildwheel due to manylinux images not having
# libpython*.so, so this is tested manually in the CI
test-command = "true"

[tool.cibuildwheel.windows]
environment = { SKBUILD_CMAKE_ARGS = "-DNMODL_BUILD_WHEEL=ON;-DFLEX_INCLUDE_PATH=C:/ProgramData/chocolatey/lib/winflexbison3/tools" }
20 changes: 3 additions & 17 deletions src/codegen/codegen_coreneuron_cpp_visitor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -66,26 +66,12 @@ std::string CodegenCoreneuronCppVisitor::simulator_name() {


int CodegenCoreneuronCppVisitor::position_of_float_var(const std::string& name) const {
int index = 0;
for (const auto& var: codegen_float_variables) {
if (var->get_name() == name) {
return index;
}
index += var->get_length();
}
throw std::logic_error(name + " variable not found");
return get_prefixsum_from_name(codegen_float_variables, name);
}


int CodegenCoreneuronCppVisitor::position_of_int_var(const std::string& name) const {
int index = 0;
for (const auto& var: codegen_int_variables) {
if (var.symbol->get_name() == name) {
return index;
}
index += var.symbol->get_length();
}
throw std::logic_error(name + " variable not found");
return get_prefixsum_from_name(codegen_int_variables, name);
}


Expand Down Expand Up @@ -418,7 +404,7 @@ void CodegenCoreneuronCppVisitor::print_check_table_thread_function() {
printer->add_line("double v = 0;");

for (const auto& function: info.functions_with_table) {
auto method_name_str = method_name(table_function_prefix() + function->get_node_name());
auto method_name_str = table_update_function_name(function->get_node_name());
auto arguments = internal_method_arguments();
printer->fmt_line("{}({});", method_name_str, arguments);
}
Expand Down
27 changes: 7 additions & 20 deletions src/codegen/codegen_cpp_visitor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,8 @@ bool CodegenCppVisitor::has_parameter_of_name(const T& node, const std::string&
}


std::string CodegenCppVisitor::table_function_prefix() const {
return "lazy_update_";
std::string CodegenCppVisitor::table_update_function_name(const std::string& block_name) const {
return "update_table_" + method_name(block_name);
}


Expand Down Expand Up @@ -404,17 +404,7 @@ std::pair<std::string, std::string> CodegenCppVisitor::write_ion_variable_name(


int CodegenCppVisitor::get_int_variable_index(const std::string& var_name) {
auto it = std::find_if(codegen_int_variables.cbegin(),
codegen_int_variables.cend(),
[&var_name](const auto& int_var) {
return int_var.symbol->get_name() == var_name;
});

if (it == codegen_int_variables.cend()) {
throw std::runtime_error(fmt::format("Unknown int variable: {}", var_name));
}

return static_cast<int>(it - codegen_int_variables.cbegin());
return get_index_from_name(codegen_int_variables, var_name);
}


Expand Down Expand Up @@ -473,9 +463,7 @@ void CodegenCppVisitor::print_function_call(const FunctionCall& node) {
}
return {name, false};
};
std::string function_name;
bool is_random_function;
std::tie(function_name, is_random_function) = get_renamed_random_function(name);
auto [function_name, is_random_function] = get_renamed_random_function(name);

if (defined_method(name)) {
function_name = method_name(name);
Expand Down Expand Up @@ -1353,7 +1341,7 @@ void CodegenCppVisitor::setup(const Program& node) {
info.mod_file = mod_filename;

if (info.mod_suffix == "") {
info.mod_suffix = std::filesystem::path(mod_filename).stem();
info.mod_suffix = std::filesystem::path(mod_filename).stem().string();
}
info.rsuffix = info.point_process ? "" : "_" + info.mod_suffix;

Expand Down Expand Up @@ -1524,9 +1512,8 @@ void CodegenCppVisitor::print_table_check_function(const Block& node) {
auto float_type = default_float_data_type();

printer->add_newline(2);
printer->fmt_push_block("void {}{}({})",
table_function_prefix(),
method_name(name),
printer->fmt_push_block("void {}({})",
table_update_function_name(name),
get_parameter_str(internal_params));
{
printer->fmt_push_block("if ({} == 0)", use_table_var);
Expand Down
49 changes: 46 additions & 3 deletions src/codegen/codegen_cpp_visitor.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,47 @@ struct ShadowUseStatement {
std::string rhs;
};

inline std::string get_name(const std::shared_ptr<symtab::Symbol>& sym) {
return sym->get_name();
}

inline std::string get_name(const IndexVariableInfo& var) {
return var.symbol->get_name();
}

template <class T>
int get_index_from_name(const std::vector<T>& variables, const std::string& name) {
auto it = std::find_if(variables.cbegin(), variables.cend(), [&name](const auto& var) {
return get_name(var) == name;
});

if (it == variables.cend()) {
throw std::runtime_error(fmt::format("Unknown variable: {}", name));
}

return static_cast<int>(it - variables.cbegin());
}

inline int get_length(const std::shared_ptr<symtab::Symbol>& sym) {
return sym->get_length();
}

inline int get_length(const IndexVariableInfo& var) {
return var.symbol->get_length();
}

template <class T>
int get_prefixsum_from_name(const std::vector<T>& variables, const std::string& name) {
int index = 0;
for (const auto& var: variables) {
if (get_name(var) == name) {
return index;
}
index += get_length(var);
}
throw std::logic_error(name + " variable not found");
}

/** \} */ // end of codegen_details


Expand Down Expand Up @@ -1112,10 +1153,12 @@ class CodegenCppVisitor: public visitor::ConstAstVisitor {
bool use_instance = true) const = 0;

/**
* Prefix used for the function that performs the lazy update
* The name of the function that updates the table value if the parameters
* changed.
*
* \param block_name The name of the block that contains the TABLE.
*/
std::string table_function_prefix() const;

std::string table_update_function_name(const std::string& block_name) const;

/**
* Return ion variable name and corresponding ion read variable name.
Expand Down
Loading

0 comments on commit d97d88c

Please sign in to comment.