From b97f74e202ad36769a9f36db06ac7f739eadb52d Mon Sep 17 00:00:00 2001 From: Phil Miller Date: Tue, 26 Dec 2023 16:51:53 -0800 Subject: [PATCH 01/10] Move pybind build out of the way so that extern/ can be linked --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 281990fa03..be62588052 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -174,7 +174,7 @@ if(NGEN_WITH_PYTHON) add_compile_definitions(ACTIVATE_PYTHON=true) find_package(Python 3.6.8 REQUIRED COMPONENTS Interpreter Development NumPy) set(PYTHON_EXECUTABLE ${Python_EXECUTABLE}) # Case-sensitive difference - add_subdirectory(extern/pybind11) + add_subdirectory(extern/pybind11 pybind11) endif() # ----------------------------------------------------------------------------- From fc3c7db8153dddd9b4b7599231c0c09b1b995924 Mon Sep 17 00:00:00 2001 From: Phil Miller Date: Tue, 26 Dec 2023 16:52:32 -0800 Subject: [PATCH 02/10] Symlink NGEN_ROOT_DIR/extern/ into the build tree so that tests can reference it easily --- test/CMakeLists.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 6b4de5288a..2576db9139 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -17,6 +17,9 @@ if(NOT NGEN_ROOT_DIR STREQUAL PROJECT_BINARY_DIR) # Create a symlink between the test data directory in the source tree and the build tree file(CREATE_LINK "${CMAKE_CURRENT_LIST_DIR}/data" "${PROJECT_BINARY_DIR}/test/data" SYMBOLIC) + + # Create a symlink between the extern directory in the source tree and the build tree + file(CREATE_LINK "${NGEN_ROOT_DIR}/extern" "${PROJECT_BINARY_DIR}/extern" SYMBOLIC) endif() # ============================================================================= From 11bc58e941ff7b613189aaf0938f43dbc5b582ad Mon Sep 17 00:00:00 2001 From: Phil Miller Date: Tue, 26 Dec 2023 16:52:49 -0800 Subject: [PATCH 03/10] Fix stupid trailing whitespace --- include/bmi/AbstractCLibBmiAdapter.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/bmi/AbstractCLibBmiAdapter.hpp b/include/bmi/AbstractCLibBmiAdapter.hpp index 6e7e23823f..773419879d 100644 --- a/include/bmi/AbstractCLibBmiAdapter.hpp +++ b/include/bmi/AbstractCLibBmiAdapter.hpp @@ -98,7 +98,7 @@ namespace models { this->init_exception_msg = "Can't init " + this->model_name + "; library file path is empty"; throw std::runtime_error(this->init_exception_msg); - } + } if(bmi_lib_file.substr(idx) == ".so"){ alt_bmi_lib_file = bmi_lib_file.substr(0,idx) + ".dylib"; } else if(bmi_lib_file.substr(idx) == ".dylib"){ From a94b1a156d27878900186d43bdf060d3c6d8f2ba Mon Sep 17 00:00:00 2001 From: Phil Miller Date: Tue, 26 Dec 2023 17:16:25 -0800 Subject: [PATCH 04/10] WIP: Modify tests to reflect that external file dependencies are linked in the build tree, and don't need to be located relative to the source path --- test/bmi/Bmi_Py_Adapter_Test.cpp | 14 ++++++++------ .../catchments/Bmi_Multi_Formulation_Test.cpp | 4 ++-- .../catchments/Bmi_Py_Formulation_Test.cpp | 14 ++++++++------ 3 files changed, 18 insertions(+), 14 deletions(-) diff --git a/test/bmi/Bmi_Py_Adapter_Test.cpp b/test/bmi/Bmi_Py_Adapter_Test.cpp index 9ce706f710..64af3e0621 100644 --- a/test/bmi/Bmi_Py_Adapter_Test.cpp +++ b/test/bmi/Bmi_Py_Adapter_Test.cpp @@ -129,12 +129,12 @@ py::object Bmi_Py_Adapter_Test::Path = InterpreterUtil::getPyModule(std::vector< void Bmi_Py_Adapter_Test::SetUp() { - std::string repo_root = py_find_repo_root(); + //std::string repo_root = py_find_repo_root(); example_scenario template_ex_struct; // These should be safe for all examples template_ex_struct.module_name = "test_bmi_py.bmi_model"; - template_ex_struct.module_directory = repo_root + "/extern/"; + template_ex_struct.module_directory = "./extern/"; // Now generate the examples vector based on the above template example size_t num_example_scenarios = 1; @@ -143,11 +143,11 @@ void Bmi_Py_Adapter_Test::SetUp() { examples[i] = template_ex_struct; } - examples[0].forcing_file = repo_root + "/data/forcing/cat-27_2015-12-01 00_00_00_2015-12-30 23_00_00.csv"; + examples[0].forcing_file = "./data/forcing/cat-27_2015-12-01 00_00_00_2015-12-30 23_00_00.csv"; // We can handle setting the right init config and initializing the adapter in a loop for (int i = 0; i < examples.size(); ++i) { - examples[i].bmi_init_config = repo_root + "/test/data/bmi/test_bmi_python/test_bmi_python_config_" + examples[i].bmi_init_config = "./test/data/bmi/test_bmi_python/test_bmi_python_config_" + std::to_string(i) + ".yml"; examples[i].adapter = std::make_shared(examples[i].module_name, examples[i].bmi_init_config, @@ -160,14 +160,16 @@ void Bmi_Py_Adapter_Test::TearDown() { } void Bmi_Py_Adapter_Test::SetUpTestSuite() { - std::string repo_root = py_find_repo_root(); - std::string module_directory = repo_root + "/extern/"; + //std::string repo_root = py_find_repo_root(); + std::string module_directory = "./extern/"; + #if 0 // Add the package dir from a local virtual environment directory also, if there is one std::string venv_dir = py_dir_search({repo_root + "/.venv", repo_root + "/venv"}); if (!venv_dir.empty()) { InterpreterUtil::addToPyPath(py_find_venv_site_packages_dir(venv_dir)); } + #endif // Also add the extern dir with our test lib to Python system path InterpreterUtil::addToPyPath(module_directory); } diff --git a/test/realizations/catchments/Bmi_Multi_Formulation_Test.cpp b/test/realizations/catchments/Bmi_Multi_Formulation_Test.cpp index c986cef2f4..1f741f25f9 100644 --- a/test/realizations/catchments/Bmi_Multi_Formulation_Test.cpp +++ b/test/realizations/catchments/Bmi_Multi_Formulation_Test.cpp @@ -467,8 +467,8 @@ std::shared_ptr Bmi_Multi_Formulation_Test::interperter = Inter void Bmi_Multi_Formulation_Test::SetUpTestSuite() { #ifdef ACTIVATE_PYTHON - std::string repo_root = py_find_repo_root(); - std::string module_directory = repo_root + "/extern/"; + // std::string repo_root = py_find_repo_root(); + std::string module_directory = "./extern/"; // Add the extern dir with our test lib to Python system path InterpreterUtil::addToPyPath(module_directory); diff --git a/test/realizations/catchments/Bmi_Py_Formulation_Test.cpp b/test/realizations/catchments/Bmi_Py_Formulation_Test.cpp index 9def1547b1..bd84e3af03 100644 --- a/test/realizations/catchments/Bmi_Py_Formulation_Test.cpp +++ b/test/realizations/catchments/Bmi_Py_Formulation_Test.cpp @@ -182,13 +182,13 @@ std::shared_ptr Bmi_Py_Formulation_Test::interperter = Interpre void Bmi_Py_Formulation_Test::SetUp() { Path = InterpreterUtil::getPyModule(std::vector {"pathlib", "Path"}); - std::string repo_root = py_find_repo_root(); - std::string forcing_data_dir = repo_root + "/data/forcing/"; + //std::string repo_root = py_find_repo_root(); + std::string forcing_data_dir = "./data/forcing/"; py_formulation_example_scenario template_ex_struct; // These should be safe for all examples template_ex_struct.module_name = "test_bmi_py.bmi_model"; - template_ex_struct.module_directory = repo_root + "/extern/"; + template_ex_struct.module_directory = "./extern/"; template_ex_struct.main_output_variable = "OUTPUT_VAR_1"; template_ex_struct.uses_forcing_file = false; @@ -205,7 +205,7 @@ void Bmi_Py_Formulation_Test::SetUp() { // We can handle setting the rest up in a loop std::string forcing_file; for (size_t i = 0; i < examples.size(); ++i) { - examples[i].bmi_init_config = repo_root + "/test/data/bmi/test_bmi_python/test_bmi_python_config_" + examples[i].bmi_init_config = "./test/data/bmi/test_bmi_python/test_bmi_python_config_" + std::to_string(i) + ".yml"; forcing_file = forcing_data_dir + examples[i].catchment_id + "_2015-12-01 00_00_00_2015-12-30 23_00_00.csv"; examples[i].forcing_params = std::make_shared(forcing_file, "legacy", "2015-12-01 00:00:00", @@ -226,14 +226,16 @@ void Bmi_Py_Formulation_Test::SetUp() { } void Bmi_Py_Formulation_Test::SetUpTestSuite() { - std::string repo_root = py_find_repo_root(); - std::string module_directory = repo_root + "/extern/"; + // std::string repo_root = py_find_repo_root(); + std::string module_directory = "./extern/"; + #if 0 // Add the package dir from a local virtual environment directory also, if there is one std::string venv_dir = py_dir_search({repo_root + "/.venv", repo_root + "/venv"}); if (!venv_dir.empty()) { InterpreterUtil::addToPyPath(py_find_venv_site_packages_dir(venv_dir)); } + #endif // Also add the extern dir with our test lib to Python system path InterpreterUtil::addToPyPath(module_directory); From 5fa4ea7cd1ea41642a3d4f882439d739df84580f Mon Sep 17 00:00:00 2001 From: Phil Miller Date: Wed, 27 Dec 2023 10:17:35 -0800 Subject: [PATCH 05/10] Run python tests from root of build tree, so ./extern/ is correct --- .github/workflows/test_and_validate.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test_and_validate.yml b/.github/workflows/test_and_validate.yml index ff1cae9a34..acccc9d6d4 100644 --- a/.github/workflows/test_and_validate.yml +++ b/.github/workflows/test_and_validate.yml @@ -290,8 +290,8 @@ jobs: - name: run_bmi_python_tests run: | . .venv/bin/activate - cd ./cmake_build/test/ - ./test_bmi_python + cd ./cmake_build/ + ./test/test_bmi_python cd ../../ timeout-minutes: 15 From 28f2f870a8c3247b34ee5637971836c4a4f34c83 Mon Sep 17 00:00:00 2001 From: Phil Miller Date: Wed, 27 Dec 2023 10:24:53 -0800 Subject: [PATCH 06/10] Py_Adapter_Test: remove uses of py_find_repo_root --- test/bmi/Bmi_Py_Adapter_Test.cpp | 30 ------------------------------ 1 file changed, 30 deletions(-) diff --git a/test/bmi/Bmi_Py_Adapter_Test.cpp b/test/bmi/Bmi_Py_Adapter_Test.cpp index 64af3e0621..d051fbdd22 100644 --- a/test/bmi/Bmi_Py_Adapter_Test.cpp +++ b/test/bmi/Bmi_Py_Adapter_Test.cpp @@ -87,13 +87,6 @@ class Bmi_Py_Adapter_Test : public ::testing::Test { */ static std::string py_dir_search(const std::vector &dir_options); - /** - * Find the repo root directory, starting from the current directory and working upward. - * - * @return The absolute path of the repo root, as a string. - */ - static std::string py_find_repo_root(); - /** * Find the virtual environment site packages directory, starting from an assumed valid venv directory. * @@ -128,9 +121,6 @@ std::shared_ptr Bmi_Py_Adapter_Test::interperter = InterpreterU py::object Bmi_Py_Adapter_Test::Path = InterpreterUtil::getPyModule(std::vector {"pathlib", "Path"}); void Bmi_Py_Adapter_Test::SetUp() { - - //std::string repo_root = py_find_repo_root(); - example_scenario template_ex_struct; // These should be safe for all examples template_ex_struct.module_name = "test_bmi_py.bmi_model"; @@ -160,7 +150,6 @@ void Bmi_Py_Adapter_Test::TearDown() { } void Bmi_Py_Adapter_Test::SetUpTestSuite() { - //std::string repo_root = py_find_repo_root(); std::string module_directory = "./extern/"; #if 0 @@ -234,25 +223,6 @@ std::string Bmi_Py_Adapter_Test::py_dir_search(const std::vector &d return ""; } -/** - * Find the repo root directory, starting from the current directory and working upward. - * - * @return The absolute path of the repo root, as a string. - */ -std::string Bmi_Py_Adapter_Test::py_find_repo_root() { - py::object dir = Path(".").attr("resolve")(); - while (!dir.equal(dir.attr("parent"))) { - // If there is a child .git dir and a child .github dir, then dir is the root - py::bool_ is_git_dir = py::bool_(dir.attr("joinpath")(".git").attr("is_dir")()); - py::bool_ is_github_dir = py::bool_(dir.attr("joinpath")(".github").attr("is_dir")()); - if (is_git_dir && is_github_dir) { - return py::str(dir); - } - dir = dir.attr("parent"); - } - throw std::runtime_error("Can't find repo root starting at " + std::string(py::str(Path(".").attr("resolve")()))); -} - /** * Find the virtual environment site packages directory, starting from an assumed valid venv directory. * From ee853c4a9dc435dba12a4a844835954574e4b584 Mon Sep 17 00:00:00 2001 From: Phil Miller Date: Wed, 27 Dec 2023 10:25:47 -0800 Subject: [PATCH 07/10] Bmi_Py_Formulation_Test: remove uses of py_find_repo_root --- .../catchments/Bmi_Py_Formulation_Test.cpp | 29 ------------------- 1 file changed, 29 deletions(-) diff --git a/test/realizations/catchments/Bmi_Py_Formulation_Test.cpp b/test/realizations/catchments/Bmi_Py_Formulation_Test.cpp index bd84e3af03..f070033de0 100644 --- a/test/realizations/catchments/Bmi_Py_Formulation_Test.cpp +++ b/test/realizations/catchments/Bmi_Py_Formulation_Test.cpp @@ -97,13 +97,6 @@ class Bmi_Py_Formulation_Test : public ::testing::Test { return diff.ticks() / boost::posix_time::time_duration::rep_type::ticks_per_second; } - /** - * Find the repo root directory, starting from the current directory and working upward. - * - * @return The absolute path of the repo root, as a string. - */ - static std::string py_find_repo_root(); - /** * Search for and return the first existing example of a collection of directories, using Python to perform search. * @@ -182,7 +175,6 @@ std::shared_ptr Bmi_Py_Formulation_Test::interperter = Interpre void Bmi_Py_Formulation_Test::SetUp() { Path = InterpreterUtil::getPyModule(std::vector {"pathlib", "Path"}); - //std::string repo_root = py_find_repo_root(); std::string forcing_data_dir = "./data/forcing/"; py_formulation_example_scenario template_ex_struct; @@ -226,7 +218,6 @@ void Bmi_Py_Formulation_Test::SetUp() { } void Bmi_Py_Formulation_Test::SetUpTestSuite() { - // std::string repo_root = py_find_repo_root(); std::string module_directory = "./extern/"; #if 0 @@ -353,26 +344,6 @@ std::string Bmi_Py_Formulation_Test::py_dir_search(const std::vector {"pathlib", "Path"}); - py::object dir = Path(".").attr("resolve")(); - while (!dir.equal(dir.attr("parent"))) { - // If there is a child .git dir and a child .github dir, then dir is the root - py::bool_ is_git_dir = py::bool_(dir.attr("joinpath")(".git").attr("is_dir")()); - py::bool_ is_github_dir = py::bool_(dir.attr("joinpath")(".github").attr("is_dir")()); - if (is_git_dir && is_github_dir) { - return py::str(dir); - } - dir = dir.attr("parent"); - } - throw std::runtime_error("Can't find repo root starting at " + std::string(py::str(Path(".").attr("resolve")()))); -} - /** * Find the virtual environment site packages directory, starting from an assumed valid venv directory. * From 3ef981e278e64365f0d9587aa3ff6a93a3dce9ce Mon Sep 17 00:00:00 2001 From: Phil Miller Date: Wed, 27 Dec 2023 10:27:35 -0800 Subject: [PATCH 08/10] Bmi_Multi_Formulation_Test: remove uses of py_find_repo_root --- .../catchments/Bmi_Multi_Formulation_Test.cpp | 27 ------------------- 1 file changed, 27 deletions(-) diff --git a/test/realizations/catchments/Bmi_Multi_Formulation_Test.cpp b/test/realizations/catchments/Bmi_Multi_Formulation_Test.cpp index 1f741f25f9..fba1d76433 100644 --- a/test/realizations/catchments/Bmi_Multi_Formulation_Test.cpp +++ b/test/realizations/catchments/Bmi_Multi_Formulation_Test.cpp @@ -403,32 +403,6 @@ class Bmi_Multi_Formulation_Test : public ::testing::Test { "formulations").begin()->second.get_child("params"); } - /** - * Find the repo root directory using Python, starting from the current directory and working upward. - * - * This will throw a runtime error if Python functionality is not active. - * - * @return The absolute path of the repo root, as a string. - */ - static std::string py_find_repo_root() { - #ifdef ACTIVATE_PYTHON - py::object Path = InterpreterUtil::getPyModule(std::vector {"pathlib", "Path"}); - py::object dir = Path(".").attr("resolve")(); - while (!dir.equal(dir.attr("parent"))) { - // If there is a child .git dir and a child .github dir, then dir is the root - py::bool_ is_git_dir = py::bool_(dir.attr("joinpath")(".git").attr("is_dir")()); - py::bool_ is_github_dir = py::bool_(dir.attr("joinpath")(".github").attr("is_dir")()); - if (is_git_dir && is_github_dir) { - return py::str(dir); - } - dir = dir.attr("parent"); - } - throw std::runtime_error("Can't find repo root starting at " + std::string(py::str(Path(".").attr("resolve")()))); - #else // (i.e., if not ACTIVATE_PYTHON) - throw std::runtime_error("Can't use Python-based test helper function 'py_find_repo_root'; Python not active!"); - #endif // ACTIVATE_PYTHON - } - inline void initializeTestExample(const int ex_index, const std::string &cat_id, const std::vector &nested_types, const std::vector &output_variables) { catchment_ids[ex_index] = cat_id; @@ -467,7 +441,6 @@ std::shared_ptr Bmi_Multi_Formulation_Test::interperter = Inter void Bmi_Multi_Formulation_Test::SetUpTestSuite() { #ifdef ACTIVATE_PYTHON - // std::string repo_root = py_find_repo_root(); std::string module_directory = "./extern/"; // Add the extern dir with our test lib to Python system path From 83b3950c042b840212cb5d211d09f889c33c9e85 Mon Sep 17 00:00:00 2001 From: Phil Miller Date: Wed, 27 Dec 2023 10:29:49 -0800 Subject: [PATCH 09/10] Drop hunting for venv at runtime in unit tests - just expect it to be active --- test/bmi/Bmi_Py_Adapter_Test.cpp | 10 +--------- .../catchments/Bmi_Py_Formulation_Test.cpp | 12 +----------- 2 files changed, 2 insertions(+), 20 deletions(-) diff --git a/test/bmi/Bmi_Py_Adapter_Test.cpp b/test/bmi/Bmi_Py_Adapter_Test.cpp index d051fbdd22..5523ca22fe 100644 --- a/test/bmi/Bmi_Py_Adapter_Test.cpp +++ b/test/bmi/Bmi_Py_Adapter_Test.cpp @@ -150,16 +150,8 @@ void Bmi_Py_Adapter_Test::TearDown() { } void Bmi_Py_Adapter_Test::SetUpTestSuite() { + // Add the extern dir with our test lib to Python system path std::string module_directory = "./extern/"; - - #if 0 - // Add the package dir from a local virtual environment directory also, if there is one - std::string venv_dir = py_dir_search({repo_root + "/.venv", repo_root + "/venv"}); - if (!venv_dir.empty()) { - InterpreterUtil::addToPyPath(py_find_venv_site_packages_dir(venv_dir)); - } - #endif - // Also add the extern dir with our test lib to Python system path InterpreterUtil::addToPyPath(module_directory); } diff --git a/test/realizations/catchments/Bmi_Py_Formulation_Test.cpp b/test/realizations/catchments/Bmi_Py_Formulation_Test.cpp index f070033de0..ab612d4810 100644 --- a/test/realizations/catchments/Bmi_Py_Formulation_Test.cpp +++ b/test/realizations/catchments/Bmi_Py_Formulation_Test.cpp @@ -218,19 +218,9 @@ void Bmi_Py_Formulation_Test::SetUp() { } void Bmi_Py_Formulation_Test::SetUpTestSuite() { + // Add the extern dir with our test lib to Python system path std::string module_directory = "./extern/"; - - #if 0 - // Add the package dir from a local virtual environment directory also, if there is one - std::string venv_dir = py_dir_search({repo_root + "/.venv", repo_root + "/venv"}); - if (!venv_dir.empty()) { - InterpreterUtil::addToPyPath(py_find_venv_site_packages_dir(venv_dir)); - } - #endif - // Also add the extern dir with our test lib to Python system path InterpreterUtil::addToPyPath(module_directory); - - } void Bmi_Py_Formulation_Test::TearDown() { From 35cea7844b3f658048e2bc67d93f459afc664a1b Mon Sep 17 00:00:00 2001 From: Phil Miller Date: Wed, 27 Dec 2023 10:55:19 -0800 Subject: [PATCH 10/10] Link extern under test/ as well --- test/CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 2576db9139..3908938c07 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -20,6 +20,7 @@ if(NOT NGEN_ROOT_DIR STREQUAL PROJECT_BINARY_DIR) # Create a symlink between the extern directory in the source tree and the build tree file(CREATE_LINK "${NGEN_ROOT_DIR}/extern" "${PROJECT_BINARY_DIR}/extern" SYMBOLIC) + file(CREATE_LINK "${NGEN_ROOT_DIR}/extern" "${PROJECT_BINARY_DIR}/test/extern" SYMBOLIC) endif() # =============================================================================