Merge pull request #4868 from NREL/4846_labs_python_native

#4846 - Allow importing Python package with C-extension e.g. numpy in the labs CLI

CMakeLists.txt

@@ -1,5 +1,5 @@
 message(STATUS "Using CMake ${CMAKE_VERSION}")
-cmake_minimum_required(VERSION 3.19.0)
+cmake_minimum_required(VERSION 3.20.0)
 cmake_policy(SET CMP0048 NEW)
 
 set(CMAKE_CXX_STANDARD 20)

python/SetupPython.cmake

@@ -7,9 +7,9 @@ set(Python_USE_STATIC_LIBS OFF)
 # find_package(Python) has the problem that on github actions in particular it'll pick up the most recent python (eg 3.9) from the tool cache
 # even if you have used the setup-python action and set it to 3.8
 if (PYTHON_VERSION)
-  find_package(Python ${PYTHON_VERSION} EXACT COMPONENTS Interpreter Development REQUIRED)
+  find_package(Python ${PYTHON_VERSION} EXACT REQUIRED COMPONENTS Interpreter Development OPTIONAL_COMPONENTS NumPy)
 else()
-  find_package(Python COMPONENTS Interpreter Development REQUIRED)
+  find_package(Python REQUIRED COMPONENTS Interpreter Development OPTIONAL_COMPONENTS NumPy)
 endif()
 
 execute_process(COMMAND ${Python_EXECUTABLE} -m pytest --version

python/engine/PythonEngine.cpp

@@ -30,9 +30,12 @@ void addToPythonPath(const openstudio::path& includePath) {
   if (!includePath.empty()) {
     PyObject* sys = PyImport_ImportModule("sys");
     PyObject* sysPath = PyObject_GetAttrString(sys, "path");
+    Py_DECREF(sys);  // PyImport_ImportModule returns a new reference, decrement it
+
     // fmt::print("Prepending '{}' to sys.path\n", includePath);
     PyObject* unicodeIncludePath = PyUnicode_FromString(includePath.string().c_str());
     PyList_Insert(sysPath, 0, unicodeIncludePath);
+    Py_DECREF(sysPath);  // PyObject_GetAttrString returns a new reference, decrement it
   }
 }
 
@@ -41,30 +44,56 @@ void PythonEngine::pyimport(const std::string& importName, const std::string& in
   PyImport_ImportModule(importName.c_str());
 }
 
-void PythonEngine::setupPythonPath(const std::vector<openstudio::path>& includeDirs, const openstudio::path& pythonHomeDir) {
-  for (const auto& includeDir : includeDirs) {
-    addToPythonPath(includeDir);
-  }
-  if (!pythonHomeDir.empty()) {
-    wchar_t* a = Py_DecodeLocale(pythonHomeDir.generic_string().c_str(), nullptr);
-    Py_SetPythonHome(a);
+void PythonEngine::setupPythonPath(const std::vector<openstudio::path>& includeDirs) {
+
+  // Iterate in reverse order since addToPythonPath always inserts at pos 0
+  // --python_path path1 --python_path path2  =>  includeDirs = ["path1", "path2"]
+  // std::ranges::reverse_view needs modern compilers
+  for (auto it = includeDirs.rbegin(); it != includeDirs.rend(); it++) {
+    addToPythonPath(*it);
   }
 }
 
 PythonEngine::PythonEngine(int argc, char* argv[]) : ScriptEngine(argc, argv), program(Py_DecodeLocale(pythonProgramName, nullptr)) {
+  // TODO: modernize and use PyConfig (new in 3.8): https://docs.python.org/3/c-api/init_config.html
+
   // this frozen flag tells Python that the package and library have been frozen for embedding, so it shouldn't warn about missing prefixes
   Py_FrozenFlag = 1;
 
-  // Set the PYTHONPATH / PYTHONHOME to the E+ shipped standard library
-  // I think we need to set the python path before initializing the library
+  // Path to the E+ shipped standard library
   auto pathToPythonPackages = getEnergyPlusDirectory() / "python_standard_lib";
-  wchar_t* a = Py_DecodeLocale(pathToPythonPackages.make_preferred().string().c_str(), nullptr);
-  Py_SetPath(a);
-  Py_SetPythonHome(a);
+
+  // The PYTHONPATH / PYTHONHOME should be set before initializing Python
+  // If this Py_SetPath is called before Py_Initialize, then Py_GetPath won't attempt to compute a default search path
+  // The default search path is affected by the Py_SetPythonHome
+  // * if the user passed --python_home, we use that as the Python Home, and do not use Py_SetPath. But later we add the E+ standard_lib anyways
+  //   so it takes precedence (to limit incompatibility issues...)
+  // * If the user didn't pass it, we use Py_SetPath set to the E+ standard_lib
+
+  std::vector<std::string> args(argv, std::next(argv, static_cast<std::ptrdiff_t>(argc)));
+  bool pythonHomePassed = false;
+  auto it = std::find(args.cbegin(), args.cend(), "--python_home");
+  if (it != args.cend()) {
+    openstudio::path pythonHomeDir(*std::next(it));
+    wchar_t* h = Py_DecodeLocale(pythonHomeDir.make_preferred().string().c_str(), nullptr);
+    Py_SetPythonHome(h);
+    pythonHomePassed = true;
+  } else {
+    wchar_t* a = Py_DecodeLocale(pathToPythonPackages.make_preferred().string().c_str(), nullptr);
+    Py_SetPath(a);
+  }
 
   Py_SetProgramName(program);  // optional but recommended
 
   Py_Initialize();
+
+  if (pythonHomePassed) {
+    addToPythonPath(pathToPythonPackages);
+  }
+#if defined(__APPLE__) || defined(__linux___) || defined(__unix__)
+  addToPythonPath(pathToPythonPackages / "lib-dynload");
+#endif
+
   PyObject* m = PyImport_AddModule("__main__");
   if (m == nullptr) {
     throw std::runtime_error("Unable to add module __main__ for python script execution");

python/engine/PythonEngine.hpp

@@ -30,7 +30,7 @@ class PythonEngine final : public ScriptEngine
   ScriptObject eval(std::string_view sv) override;
   void exec(std::string_view sv) override;
 
-  virtual void setupPythonPath(const std::vector<openstudio::path>& includeDirs, const openstudio::path& pythonHomeDir) override;
+  virtual void setupPythonPath(const std::vector<openstudio::path>& includeDirs) override;
 
  protected:
   void* getAs_impl(ScriptObject& obj, const std::type_info&) override;

src/cli/CMakeLists.txt

@@ -337,6 +337,61 @@ if(BUILD_TESTING)
 
   # ============ EndForward a Path properly no matter the slashes employed ============
 
+  # ============ Native Ruby Gems / Python Modules - C extensions =============
+  if (Python_NumPy_FOUND)
+
+    if(NOT EXISTS "${Python_ROOT_DIR}")
+      # Python_STDLIB: we expect it to be
+      # Unix: ~/.pyenv/versions/3.8.12/lib/python3.8 or
+      #       or on CI: /usr/lib/python3.8/ ... with numpy if install via pip3 and not apt install python3-numpy in `/usr/local/lib/python3.8/dist-packages/`
+      # Windows C:\Python38\Lib
+      cmake_path(GET Python_STDLIB PARENT_PATH Python_ROOT_DIR)
+      if(UNIX)
+        cmake_path(GET Python_ROOT_DIR PARENT_PATH Python_ROOT_DIR)
+      endif()
+    endif()
+
+    if(UNIX)
+      if(EXISTS "${Python_SITELIB}")
+        set(PYTHON_PATH "${Python_SITELIB}" "${Python_STDLIB}/lib-dynload")
+      else()
+        set(PYTHON_PATH "${Python_STDLIB}/lib-dynload")
+      endif()
+
+      if(NOT APPLE)
+        set(EXTRA_LOCAL_DIST "/usr/local/lib/python3.8/dist-packages")
+        if (EXISTS "${EXTRA_LOCAL_DIST}")
+          list(APPEND PYTHON_PATH "${EXTRA_LOCAL_DIST}")
+        endif()
+      endif()
+    else()
+      set(PYTHON_PATH "$<SHELL_PATH:${Python_SITELIB}>")
+    endif()
+
+    message(DEBUG "PYTHON_PATH=${PYTHON_PATH}")
+
+    add_test(NAME OpenStudioCLI.Labs.execute_python_script.numpy.explicit_sys_path_insert
+      COMMAND $<TARGET_FILE:openstudio> labs execute_python_script execute_python_script_with_numpy.py ${Python_STDLIB}
+      WORKING_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}/test/"
+    )
+
+    add_test(NAME OpenStudioCLI.Labs.execute_python_script.numpy.python_path
+      COMMAND $<TARGET_FILE:openstudio> labs
+                "$<$<BOOL:${PYTHON_PATH}>:--python_path;$<JOIN:${PYTHON_PATH},;--python_path;>>"
+                execute_python_script execute_python_script_with_numpy.py
+      COMMAND_EXPAND_LISTS
+      WORKING_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}/test/"
+    )
+
+    add_test(NAME OpenStudioCLI.Labs.execute_python_script.numpy.python_home
+      COMMAND $<TARGET_FILE:openstudio> labs
+                --python_home "$<SHELL_PATH:${Python_ROOT_DIR}>"
+                execute_python_script execute_python_script_with_numpy.py
+      WORKING_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}/test/"
+    )
+  else()
+    message(AUTHOR_WARNING "Cannot run the python numpy test, as numpy isn't installed on your system python")
+  endif()
 
   file(GLOB RUBY_TEST_SRC
     # find all CLI test

src/cli/main.cpp

@@ -136,28 +136,33 @@ int main(int argc, char* argv[]) {
     experimentalApp
       ->add_option("-I,--include", includeDirs, "Add additional directory to add to front of Ruby $LOAD_PATH (may be used more than once)")
       ->option_text("DIR")
+      ->check(CLI::ExistingDirectory)
       ->group(rubySpecificOptionsGroupName);
 
     std::vector<openstudio::path> gemPathDirs;
     experimentalApp
       ->add_option("--gem_path", gemPathDirs,
                    "Add additional directory to add to front of GEM_PATH environment variable (may be used more than once)")
       ->option_text("DIR")
+      ->check(CLI::ExistingDirectory)
       ->group(rubySpecificOptionsGroupName);
 
     openstudio::path gemHomeDir;
     experimentalApp->add_option("--gem_home", gemHomeDir, "Set GEM_HOME environment variable")
       ->option_text("DIR")
+      ->check(CLI::ExistingDirectory)
       ->group(rubySpecificOptionsGroupName);
 
     openstudio::path bundleGemFilePath;
     experimentalApp->add_option("--bundle", bundleGemFilePath, "Use bundler for GEMFILE")
       ->option_text("GEMFILE")
+      ->check(CLI::ExistingFile)
       ->group(rubySpecificOptionsGroupName);
 
     openstudio::path bundleGemDirPath;
     experimentalApp->add_option("--bundle_path", bundleGemDirPath, "Use bundler installed gems in BUNDLE_PATH")
       ->option_text("BUNDLE_PATH")
+      ->check(CLI::ExistingDirectory)
       ->group(rubySpecificOptionsGroupName);
 
     // std::vector<std::string>
@@ -190,16 +195,19 @@ int main(int argc, char* argv[]) {
       ->add_option("--python_path", pythonPathDirs,
                    "Add additional directory to add to front of PYTHONPATH environment variable (may be used more than once)")
       ->option_text("DIR")
+      ->check(CLI::ExistingDirectory)
       ->group(pythonSpecificOptionsGroupName);
 
     openstudio::path pythonHomeDir;
     experimentalApp->add_option("--python_home", pythonHomeDir, "Set PYTHONHOME environment variable")
       ->option_text("DIR")
+      ->check(CLI::ExistingDirectory)
       ->group(pythonSpecificOptionsGroupName);
 
     // This is a callback that's stored on the ScriptEngineInstance, triggered only the first time
-    std::function<void()> runSetupPythonPath = [&pythonEngine, &pythonPathDirs, &pythonHomeDir]() {
-      pythonEngine->setupPythonPath(pythonPathDirs, pythonHomeDir);
+    std::function<void()> runSetupPythonPath = [&pythonEngine, &pythonPathDirs]() {
+      // pythonHomeDir is retrieved from (argc, argv) actually, as Py_SetPythonHome has to be called before Py_Initialize
+      pythonEngine->setupPythonPath(pythonPathDirs);
       pythonEngine->registerType<openstudio::measure::ModelMeasure*>("openstudio::measure::ModelMeasure *");
       pythonEngine->registerType<openstudio::measure::EnergyPlusMeasure*>("openstudio::measure::EnergyPlusMeasure *");
       pythonEngine->registerType<openstudio::measure::ReportingMeasure*>("openstudio::measure::ReportingMeasure *");

src/cli/test/execute_python_script_with_numpy.py

@@ -0,0 +1,49 @@
+"""Test for importing a python package with a C extension, numpy here."""
+
+import sys
+from pathlib import Path
+
+
+def test_import_numpy():
+    """Common test function."""
+    print(f"{sys.path=}")
+    import numpy as np
+
+    a = np.array([0, 1, 2, 3])
+    assert a.sum() == 6
+
+
+def test_with_sys_path_explicit(std_lib: Path):
+    """Explicitly insert site-packages and lib-dynload in sys.path."""
+    site_pack = std_lib / "site-packages"
+    if site_pack.exists():
+        sys.path.insert(0, str(site_pack))
+    # Only for unix
+    dynload = std_lib / "lib-dynload"
+    if dynload.exists():
+        sys.path.insert(0, str(dynload))
+    # Only for debian
+    dist_pack = std_lib.parent.parent / 'local/lib/python3.8/dist-packages'
+    if dist_pack.exists():
+        sys.path.insert(0, str(dist_pack))
+
+    test_import_numpy()
+
+
+def test_with_pythonpath_pythonhome():
+    """Rely on passing PYTHONPATH/PYTHONHOME."""
+    test_import_numpy()
+
+
+if __name__ == "__main__":
+    if len(sys.argv) == 2:
+        std_lib = Path(sys.argv[1])
+        print("Passed {std_lib=}")
+
+        if std_lib.is_dir():
+            test_with_sys_path_explicit(std_lib)
+        else:
+            raise ValueError(f"Python std_lib doesn't exist at {std_lib}")
+
+    else:
+        test_with_pythonpath_pythonhome()

src/scriptengine/ScriptEngine.hpp

@@ -61,7 +61,7 @@ class ScriptEngine
                                  const openstudio::path& gemHomeDir, const openstudio::path& bundleGemFilePath,
                                  const openstudio::path& bundleGemDirPath, const std::string& bundleWithoutGroups){};
 
-  virtual void setupPythonPath(const std::vector<openstudio::path>& includeDirs, const openstudio::path& pythonHomeDir){};
+  virtual void setupPythonPath(const std::vector<openstudio::path>& includeDirs){};
 
   template <typename T>
   T getAs(ScriptObject& obj) {

src/utilities/core/DynamicLibraryPOSIX.hpp

@@ -31,8 +31,17 @@ struct DynamicLibrary
     return symbol;
   }
 
-  explicit DynamicLibrary(openstudio::path location)
-    : m_location{std::move(location)}, m_handle{dlopen(m_location.c_str(), RTLD_LAZY | RTLD_LOCAL), m_handle_deleter} {
+  explicit DynamicLibrary(openstudio::path location) : m_location{std::move(location)} {
+    int flags = RTLD_LAZY | RTLD_LOCAL;  // NOLINT(misc-const-correctness, hicpp-signed-bitwise)
+
+    // This seems to work on Mac without RTLD_GLOBAL...
+#ifdef __linux__
+    if (m_location.filename().generic_string().find("python") != std::string::npos) {
+      // https://stackoverflow.com/questions/67891197/ctypes-cpython-39-x86-64-linux-gnu-so-undefined-symbol-pyfloat-type-in-embedd
+      flags = RTLD_LAZY | RTLD_GLOBAL;
+    }
+#endif
+    m_handle = {dlopen(m_location.c_str(), flags), m_handle_deleter};
     if (!m_handle) {
       throw std::runtime_error(fmt::format("Unable to load library '{}', reason: '{}'", m_location.string(), dlerror()));
     }