diff --git a/.github/workflows/nmodl-ci.yml b/.github/workflows/nmodl-ci.yml index 907028796b..09a2a4d20d 100644 --- a/.github/workflows/nmodl-ci.yml +++ b/.github/workflows/nmodl-ci.yml @@ -32,10 +32,10 @@ jobs: flag_warnings: ON glibc_asserts: ON os: ubuntu-22.04 - - config: - os: macos-12 - config: os: macos-13 + - config: + os: macos-14 - config: os: windows-latest # TODO: might be interesting to add the thread sanitizer too @@ -72,7 +72,7 @@ jobs: brew install python@3.12 || true brew link --overwrite python@3.12 brew install ccache coreutils bison boost flex ninja - echo /usr/local/opt/flex/bin:/usr/local/opt/bison/bin >> $GITHUB_PATH + echo /usr/local/opt/flex/bin:/usr/local/opt/bison/bin:/opt/homebrew/opt/flex/bin:/opt/homebrew/opt/bison/bin >> $GITHUB_PATH # Taken from https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners#supported-runners-and-hardware-resources echo CMAKE_BUILD_PARALLEL_LEVEL=3 >> $GITHUB_ENV shell: bash @@ -188,7 +188,7 @@ jobs: - name: Build Windows if: startsWith(matrix.config.os, 'windows') run: | - cmake --build build + cmake --build build --verbose - name: Build Linux/MacOS if: ${{ !startsWith(matrix.config.os, 'windows') }} @@ -209,7 +209,7 @@ jobs: ccache -z # Older versions don't support -v (verbose) ccache -vs 2>/dev/null || ccache -s - cmake --build build + cmake --build build --verbose ccache -vs 2>/dev/null || ccache -s - name: Test shell: bash diff --git a/.github/workflows/sonarsource.yml b/.github/workflows/sonarsource.yml index dcc03d5ee6..90211caae3 100644 --- a/.github/workflows/sonarsource.yml +++ b/.github/workflows/sonarsource.yml @@ -47,7 +47,7 @@ jobs: - name: Run build-wrapper working-directory: ${{runner.workspace}}/nmodl run: | - build-wrapper-linux-x86-64 --out-dir ${{ env.BUILD_WRAPPER_OUT_DIR }} cmake --build build/ + build-wrapper-linux-x86-64 --out-dir ${{ env.BUILD_WRAPPER_OUT_DIR }} cmake --build build --verbose --parallel - name: Run sonar-scanner continue-on-error: true env: diff --git a/CMakeLists.txt b/CMakeLists.txt index 5850e6d1bb..47b0484443 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -112,7 +112,7 @@ if(NOT TARGET CLI11::CLI11) cpp_cc_git_submodule(cli11 BUILD PACKAGE CLI11 REQUIRED) endif() # We could have fmt incoming from NEURON -if(NOT TARGET fmt) +if(NOT TARGET fmt::fmt) cpp_cc_git_submodule(fmt BUILD EXCLUDE_FROM_ALL PACKAGE fmt REQUIRED) endif() # If we're building from the submodule, make sure we pass -fPIC so that we can link the code into a @@ -139,11 +139,11 @@ endif() # 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) option(SPDLOG_SYSTEM_INCLUDE "Include spdlog as a system lib" ON) +cpp_cc_git_submodule(spdlog BUILD PACKAGE spdlog REQUIRED) if(NMODL_3RDPARTY_USE_SPDLOG) # See above, same logic as fmt - option(SPDLOG_BUILD_PIC "Needed to be PIC to be put compiled as a lib" ON) + set_target_properties(spdlog PROPERTIES POSITION_INDEPENDENT_CODE ON) endif() -cpp_cc_git_submodule(spdlog BUILD PACKAGE spdlog REQUIRED) if(NMODL_ENABLE_BACKWARD) cpp_cc_git_submodule(backward BUILD EXCLUDE_FROM_ALL PACKAGE backward REQUIRED) diff --git a/python/nmodl/ode.py b/python/nmodl/ode.py index deedc8da63..cd6b2b27ae 100644 --- a/python/nmodl/ode.py +++ b/python/nmodl/ode.py @@ -573,7 +573,13 @@ def forwards_euler2c(diff_string, dt_var, vars, function_calls): return f"{sp.ccode(x)} = {sp.ccode(solution, user_functions=custom_fcts)}" -def differentiate2c(expression, dependent_var, vars, prev_expressions=None): +def differentiate2c( + expression, + dependent_var, + vars, + prev_expressions=None, + stepsize=1e-3, +): """Analytically differentiate supplied expression, return solution as C code. Expression should be of the form "f(x)", where "x" is @@ -600,11 +606,15 @@ def differentiate2c(expression, dependent_var, vars, prev_expressions=None): vars: set of all other variables used in expression, e.g. {"a", "b", "c"} prev_expressions: time-ordered list of preceeding expressions to evaluate & substitute, e.g. ["b = x + c", "a = 12*b"] + stepsize: in case an analytic expression is not possible, finite differences are used; + this argument sets the step size Returns: string containing analytic derivative of expression (including any substitutions of variables from supplied prev_expressions) w.r.t. dependent_var as C code. """ + if stepsize <= 0: + raise ValueError("arg `stepsize` must be > 0") prev_expressions = prev_expressions or [] # every symbol (a.k.a variable) that SymPy # is going to manipulate needs to be declared @@ -648,15 +658,27 @@ def differentiate2c(expression, dependent_var, vars, prev_expressions=None): # differentiate w.r.t. x diff = expr.diff(x).simplify() + # could be something generic like f'(x), in which case we use finite differences + if needs_finite_differences(diff): + diff = ( + transform_expression(diff, discretize_derivative) + .subs({finite_difference_step_variable(x): stepsize}) + .evalf() + ) + + # the codegen method does not like undefined function calls, so we extract + # them here + custom_fcts = {str(f.func): str(f.func) for f in diff.atoms(sp.Function)} + # try to simplify expression in terms of existing variables # ignore any exceptions here, since we already have a valid solution # so if this further simplification step fails the error is not fatal try: # if expression is equal to one of the supplied vars, replace with this var # can do a simple string comparison here since a var cannot be further simplified - diff_as_string = sp.ccode(diff) + diff_as_string = sp.ccode(diff, user_functions=custom_fcts) for v in sympy_vars: - if diff_as_string == sp.ccode(sympy_vars[v]): + if diff_as_string == sp.ccode(sympy_vars[v], user_functions=custom_fcts): diff = sympy_vars[v] # or if equal to rhs of one of the supplied equations, replace with lhs @@ -677,4 +699,4 @@ def differentiate2c(expression, dependent_var, vars, prev_expressions=None): pass # return result as C code in NEURON format - return sp.ccode(diff.evalf()) + return sp.ccode(diff.evalf(), user_functions=custom_fcts) diff --git a/src/codegen/codegen_cpp_visitor.cpp b/src/codegen/codegen_cpp_visitor.cpp index 37f0c4a609..febc1ba157 100644 --- a/src/codegen/codegen_cpp_visitor.cpp +++ b/src/codegen/codegen_cpp_visitor.cpp @@ -198,12 +198,12 @@ int CodegenCppVisitor::int_variables_size() const { * representation (1e+20, 1E-15) then keep it as it is. */ std::string CodegenCppVisitor::format_double_string(const std::string& s_value) { - return utils::format_double_string(s_value); + return utils::format_double_string(s_value); } std::string CodegenCppVisitor::format_float_string(const std::string& s_value) { - return utils::format_float_string(s_value); + return utils::format_float_string(s_value); } @@ -1728,7 +1728,6 @@ void CodegenCppVisitor::print_rename_state_vars() const { auto rhs = get_variable_name(state_name + "0"); if (state->is_array()) { - auto size = state->get_length(); for (int i = 0; i < state->get_length(); ++i) { printer->fmt_line("{}[{}] = {};", lhs, i, rhs); } diff --git a/src/codegen/codegen_utils.cpp b/src/codegen/codegen_utils.cpp index 5f7b623239..c068b87b03 100644 --- a/src/codegen/codegen_utils.cpp +++ b/src/codegen/codegen_utils.cpp @@ -20,8 +20,7 @@ namespace utils { * they are represented in the mod file by user. If the value is in scientific * representation (1e+20, 1E-15) then keep it as it is. */ -template <> -std::string format_double_string(const std::string& s_value) { +std::string format_double_string(const std::string& s_value) { double value = std::stod(s_value); if (std::ceil(value) == value && s_value.find_first_of("eE") == std::string::npos) { return fmt::format("{:.1f}", value); @@ -30,8 +29,7 @@ std::string format_double_string(const std::string& s_value) } -template <> -std::string format_float_string(const std::string& s_value) { +std::string format_float_string(const std::string& s_value) { float value = std::stof(s_value); if (std::ceil(value) == value && s_value.find_first_of("eE") == std::string::npos) { return fmt::format("{:.1f}", value); diff --git a/src/codegen/codegen_utils.hpp b/src/codegen/codegen_utils.hpp index 4c910b36b5..489ca326fb 100644 --- a/src/codegen/codegen_utils.hpp +++ b/src/codegen/codegen_utils.hpp @@ -29,7 +29,6 @@ namespace utils { * \param s_value The double constant as string * \return The proper string to be printed in the generated file. */ -template std::string format_double_string(const std::string& s_value); @@ -43,7 +42,6 @@ std::string format_double_string(const std::string& s_value); * \param s_value The double constant as string * \return The proper string to be printed in the generated file. */ -template std::string format_float_string(const std::string& s_value); } // namespace utils diff --git a/src/main.cpp b/src/main.cpp index f12bfe35dd..61fbb3003d 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -314,8 +314,10 @@ int run_nmodl(int argc, const char* argv[]) { /// create file path for nmodl file auto filepath = [scratch_dir, modfile](const std::string& suffix) { static int count = 0; - return fmt::format( - "{}/{}.{}.{}.mod", scratch_dir, modfile, std::to_string(count++), suffix); + + auto filename = fmt::format("{}.{}.{}.mod", modfile, std::to_string(count++), suffix); + + return (std::filesystem::path(scratch_dir) / filename).string(); }; /// driver object creates lexer and parser, just call parser method @@ -328,9 +330,6 @@ int run_nmodl(int argc, const char* argv[]) { /// one whenever we run symtab visitor. bool update_symtab = false; - /// just visit the ast - AstVisitor().visit_program(*ast); - { logger->info("Running argument renaming visitor"); RenameFunctionArgumentsVisitor().visit_program(*ast); @@ -406,12 +405,10 @@ int run_nmodl(int argc, const char* argv[]) { ast_to_nmodl(*ast, filepath("ast")); if (json_ast) { - std::string file{scratch_dir}; - file += "/"; - file += modfile; - file += ".ast.json"; - logger->info("Writing AST into {}", file); - JSONVisitor(file).write(*ast); + std::filesystem::path file{scratch_dir}; + file /= modfile + ".ast.json"; + logger->info("Writing AST into {}", file.string()); + JSONVisitor(file.string()).write(*ast); } if (verbatim_rename) { diff --git a/src/utils/CMakeLists.txt b/src/utils/CMakeLists.txt index aff5d0d499..24cf674559 100644 --- a/src/utils/CMakeLists.txt +++ b/src/utils/CMakeLists.txt @@ -13,7 +13,7 @@ add_library( ${PROJECT_BINARY_DIR}/src/config/config.cpp) set_property(TARGET util PROPERTY POSITION_INDEPENDENT_CODE ON) -target_link_libraries(util PUBLIC fmt::fmt nlohmann_json::nlohmann_json spdlog::spdlog_header_only) +target_link_libraries(util PUBLIC fmt::fmt nlohmann_json::nlohmann_json spdlog::spdlog) if(NMODL_ENABLE_BACKWARD) target_link_libraries(util PRIVATE Backward::Interface) diff --git a/test/unit/CMakeLists.txt b/test/unit/CMakeLists.txt index 44d57fe91f..bdd7d51081 100644 --- a/test/unit/CMakeLists.txt +++ b/test/unit/CMakeLists.txt @@ -17,7 +17,7 @@ include_directories(${NMODL_PROJECT_SOURCE_DIR}/ext/eigen) # Common input data library # ============================================================================= add_library(test_util STATIC utils/nmodl_constructs.cpp utils/test_utils.cpp) -target_link_libraries(test_util PUBLIC spdlog::spdlog_header_only) +target_link_libraries(test_util PUBLIC spdlog::spdlog) # ============================================================================= # Common input data library diff --git a/test/unit/codegen/codegen_utils.cpp b/test/unit/codegen/codegen_utils.cpp index 0438cf38c3..a125f6185f 100644 --- a/test/unit/codegen/codegen_utils.cpp +++ b/test/unit/codegen/codegen_utils.cpp @@ -21,8 +21,7 @@ SCENARIO("C codegen utility functions", "[codegen][util][c]") { std::string double_constant = "0.012345678901234567"; THEN("Codegen C Visitor prints double with same precision") { - auto nmodl_constant_result = codegen::utils::format_double_string( - double_constant); + auto nmodl_constant_result = codegen::utils::format_double_string(double_constant); REQUIRE(nmodl_constant_result == double_constant); } } @@ -33,8 +32,7 @@ SCENARIO("C codegen utility functions", "[codegen][util][c]") { std::string codegen_output = "1.0"; THEN("Codegen C Visitor prints integer as double number") { - auto nmodl_constant_result = codegen::utils::format_double_string( - double_constant); + auto nmodl_constant_result = codegen::utils::format_double_string(double_constant); REQUIRE(nmodl_constant_result == codegen_output); } } @@ -44,8 +42,7 @@ SCENARIO("C codegen utility functions", "[codegen][util][c]") { THEN("Codegen C Visitor prints doubles with scientific notation") { for (const auto& test: tests) { - REQUIRE(codegen::utils::format_double_string(test.first) == - test.second); + REQUIRE(codegen::utils::format_double_string(test.first) == test.second); } } } @@ -54,8 +51,7 @@ SCENARIO("C codegen utility functions", "[codegen][util][c]") { std::string float_constant = "0.01234567"; THEN("Codegen C Visitor prints float with same precision") { - auto nmodl_constant_result = codegen::utils::format_float_string( - float_constant); + auto nmodl_constant_result = codegen::utils::format_float_string(float_constant); REQUIRE(nmodl_constant_result == float_constant); } } @@ -66,8 +62,7 @@ SCENARIO("C codegen utility functions", "[codegen][util][c]") { std::string codegen_output = "1.0"; THEN("Codegen C Visitor prints integer as double number") { - auto nmodl_constant_result = codegen::utils::format_float_string( - float_constant); + auto nmodl_constant_result = codegen::utils::format_float_string(float_constant); REQUIRE(nmodl_constant_result == codegen_output); } } @@ -77,8 +72,7 @@ SCENARIO("C codegen utility functions", "[codegen][util][c]") { THEN("Codegen C Visitor prints doubles with scientific notation") { for (const auto& test: tests) { - REQUIRE(codegen::utils::format_float_string(test.first) == - test.second); + REQUIRE(codegen::utils::format_float_string(test.first) == test.second); } } } diff --git a/test/unit/ode/test_ode.py b/test/unit/ode/test_ode.py index f855c7d0ee..39dff272af 100644 --- a/test/unit/ode/test_ode.py +++ b/test/unit/ode/test_ode.py @@ -3,7 +3,12 @@ # # SPDX-License-Identifier: Apache-2.0 +<<<<<<< HEAD from nmodl.ode import differentiate2c, integrate2c, make_symbol +======= +from nmodl.ode import differentiate2c, integrate2c +import pytest +>>>>>>> master import sympy as sp @@ -118,6 +123,38 @@ def test_differentiate2c(): {sp.IndexedBase("s", shape=[1]), sp.IndexedBase("z", shape=[1])}, ) + result = differentiate2c( + "-f(x)", + "x", + {}, + ) + # instead of comparing the expression as a string, we convert the string + # back to an expression and compare with an explicit function + size = 100 + for index in range(size): + a, b = -5, 5 + value = (b - a) * index / size + a + pytest.approx( + float( + sp.sympify(result) + .subs(sp.Function("f"), sp.sin) + .subs({"x": value}) + .evalf() + ) + ) == float( + -sp.Derivative(sp.sin("x")) + .as_finite_difference(1e-3) + .subs({"x": value}) + .evalf() + ) + with pytest.raises(ValueError): + differentiate2c( + "-f(x)", + "x", + {}, + stepsize=-1, + ) + def test_integrate2c(): diff --git a/test/unit/units/parser.cpp b/test/unit/units/parser.cpp index 3ffd3497e1..b121782440 100644 --- a/test/unit/units/parser.cpp +++ b/test/unit/units/parser.cpp @@ -27,6 +27,7 @@ using Catch::Matchers::ContainsSubstring; // able to define complex units based on base units nmodl::parser::UnitDriver driver; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) +namespace { bool is_valid_construct(const std::string& construct) { return driver.parse_string(construct); } @@ -40,6 +41,7 @@ std::string parse_string(const std::string& unit_definition) { correctness_driver.table->print_base_units(ss); return ss.str(); } +} // namespace SCENARIO("Unit parser accepting valid units definition", "[unit][parser]") { GIVEN("A base unit") {