Added tests for Python extension and updated build system for Python …

…extension Add methods for reading files into Variables that can be used by the python extension Add debug messages for duplicate input parameters and for absent EnzymeAct when useC3 is true
cropsinsilico · Dec 19, 2024 · 2cc15b4 · 2cc15b4
1 parent 657ec3d
commit 2cc15b4
Show file tree

Hide file tree

Showing 15 changed files with 592 additions and 231 deletions.
diff --git a/.github/workflows/test-suite.yml b/.github/workflows/test-suite.yml
@@ -22,6 +22,7 @@ jobs:
         with-elf: [false]
         with-valgrind: [false, true]
         with-mpi: [false]
+        with-python: [false]
         include:
           - sundials-version: "5.7.0"
             os: ubuntu-latest
@@ -30,41 +31,55 @@ jobs:
             with-elf: false
             with-valgrind: false
             with-mpi: false
+            with-python: false
           - sundials-version: "6.7.0"
             os: ubuntu-latest
             with-coverage: false
             with-asan: false
             with-elf: false
             with-valgrind: false
             with-mpi: false
+            with-python: false
           - sundials-version: "7.1.1"
             os: ubuntu-latest
             with-coverage: false
             with-asan: false
             with-elf: false
             with-valgrind: false
             with-mpi: false
+            with-python: false
           - sundials-version: latest
             os: ubuntu-latest
             with-coverage: false
             with-asan: false
             with-elf: false
             with-valgrind: false
             with-mpi: true
+            with-python: false
           - sundials-version: "6.7.0"
             os: ubuntu-latest
             with-coverage: false
             with-asan: false
             with-elf: false
             with-valgrind: false
             with-mpi: true
+            with-python: false
           - sundials-version: "7.1.1"
             os: ubuntu-latest
             with-coverage: false
             with-asan: false
             with-elf: false
             with-valgrind: false
             with-mpi: true
+            with-python: false
+          - sundials-version: latest
+            os: ubuntu-latest
+            with-coverage: false
+            with-asan: false
+            with-elf: false
+            with-valgrind: false
+            with-mpi: false
+            with-python: true
         exclude:
           - os: windows-latest
             with-asan: true
@@ -212,6 +227,11 @@ jobs:
           echo "CMAKE_ARGS=-DCMAKE_VERBOSE_MAKEFILE:BOOL=ON" >> "$GITHUB_ENV"
           echo "CMAKE_ARGS_TEST=-DBUILD_TESTS=ON" >> "$GITHUB_ENV"
           echo "TEST_FLAGS=-C ${CMAKE_BUILD_TYPE_TEST} --output-on-failure -VV" >> "$GITHUB_ENV"
+          echo "BUILD_ARGS=--config ${CMAKE_BUILD_TYPE_TEST}" >> "$GITHUB_ENV"
+      - name: Set flags to build in parallel (UNIX)
+        if: matrix.os != 'windows-latest'
+        run: |
+          echo "BUILD_ARGS=${{ env.BUILD_ARGS }} -- -j 4" >> "$GITHUB_ENV"
       - name: Coverage config flags
         if: matrix.with-coverage == true && matrix.os != 'windows-latest'
         run: |
@@ -257,21 +277,15 @@ jobs:
           mkdir build
           cd build
           which cmake
-          cmake .. ${{ env.CMAKE_ARGS }} ${{ env.CMAKE_ARGS_NO_PYTHON }} ${{ env.CMAKE_ARGS_TEST }}
+          cmake .. ${{ env.CMAKE_ARGS }} ${{ env.CMAKE_ARGS_TEST }}
       
       ###################################
       # Build CXX
       ###################################
-      - name: Build C++ & C in parallel (CONDA, UNIX)
-        if: matrix.os != 'windows-latest'
+      - name: Build C++ & C
         run: |
           cd build
-          cmake --build . --config ${{ env.CMAKE_BUILD_TYPE_TEST }} -- -j 4
-      - name: Build C++ & C in serial (CONDA, WINDOWS)
-        if: matrix.os == 'windows-latest'
-        run: |
-          cd build
-          cmake --build . --config ${{ env.CMAKE_BUILD_TYPE_TEST }}
+          cmake --build . ${{ env.BUILD_ARGS }}
       
       ###################################
       # Test CXX
@@ -299,26 +313,25 @@ jobs:
       ###################################
       # Build Python
       ###################################
-      # - name: Set CMAKE_ARGS for Python build
-      #   run: |
-      #     echo "OLD_CMAKE_ARGS=${{ env.CMAKE_ARGS }}" >> "$GITHUB_ENV"
-      #     echo "CMAKE_ARGS=${{ env.CMAKE_ARGS }} -DBUILD_PYTHON=ON" >> "$GITHUB_ENV"
-      # - name: Build Python (CONDA)
-      #   run: |
-      #     cd build
-      #     which cmake
-      #     cmake .. ${{ env.CMAKE_ARGS }} ${{ env.CMAKE_ARGS_TEST }}
-      # - name: Unset CMAKE_ARGS for Python build
-      #   run: |
-      #     echo "CMAKE_ARGS=${{ env.OLD_CMAKE_ARGS }}" >> "$GITHUB_ENV"
+      - name: Configure Python
+        if: matrix.with-python == true
+        run: |
+          cd build
+          cmake .. -DBUILD_PYTHON=ON ${{ env.CMAKE_ARGS }}
+      - name: Build & install Python
+        if: matrix.with-python == true
+        run: |
+          cd build
+          cmake --build . --target pyPhotosynthesis ${{ env.BUILD_ARGS }}
+          cmake --install . --prefix /home/runner/miniconda3/envs/ephoto
 
       ###################################
       # Test Python
       ###################################
-      # - name: Test Python Interfaces (CONDA)
-      #   if: matrix.with-elf == false && matrix.with-valgrind == false
-      #   run: |
-      #     pytest -sv tests/python
+      - name: Test Python Interfaces
+        if: matrix.with-python == true
+        run: |
+          pytest -sv tests/python
 
       ###################################
       # Docs tests
@@ -335,7 +348,7 @@ jobs:
         if: matrix.with-coverage == true && matrix.os != 'windows-latest'
         uses: codecov/codecov-action@v4
         with:
-          name: ${{ matrix.os }}-${{ matrix.with-coverage }}-${{ matrix.with-asan }}-${{ matrix.with-elf }}-${{ matrix.with-valgrind }}
+          name: ${{ matrix.os }}-${{ matrix.sundials-version }}-${{ matrix.with-coverage }}-${{ matrix.with-asan }}-${{ matrix.with-elf }}-${{ matrix.with-valgrind }}-${{ matrix.with-mpi }}-${{ matrix.with-python }}
           token: ${{ secrets.CODECOV_TOKEN }}
           file: coverage/coverage.info
           functionalities: gcov
diff --git a/.gitignore b/.gitignore
@@ -61,4 +61,6 @@ Doxyfile
 doc/html
 doc/latex
 
-*~
+*~
+
+__pycache__
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -63,7 +63,7 @@ else()
 endif()
 # Prohibit in-source build
 if("${CMAKE_SOURCE_DIR}" STREQUAL "${CMAKE_BINARY_DIR}")
-  message(FATAL_ERROR "In-source build prohibited.")
+  message(FATAL_ERROR "In-source build prohibited.\nCMAKE_SOURCE_DIR = ${CMAKE_SOURCE_DIR}\nCMAKE_BINARY_DIR = ${CMAKE_BINARY_DIR}")
 endif("${CMAKE_SOURCE_DIR}" STREQUAL "${CMAKE_BINARY_DIR}")
 
 set(CMAKE_MODULE_PATH "${CMAKE_MODULE_PATH};${PROJECT_SOURCE_DIR}/cmake")
@@ -243,12 +243,14 @@ install(
     ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR}
     RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR}
     PUBLIC_HEADER DESTINATION EPHOTOSYNTHESIS_INSTALL_INCLUDEDIR
+    COMPONENT CXX
 )
 install(
     EXPORT ePhotosynthesisTargets
     FILE ePhotosynthesisTargets.cmake
     NAMESPACE ePhotosynthesis::
     DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/ePhotosynthesis
+    COMPONENT CXX
 )
 include(CMakePackageConfigHelpers)
 write_basic_package_version_file(
@@ -264,10 +266,12 @@ install(
     FILES "${CMAKE_CURRENT_BINARY_DIR}/ePhotosynthesisConfig.cmake"
     "${CMAKE_CURRENT_BINARY_DIR}/ePhotosynthesisConfigVersion.cmake"
     DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/ePhotosynthesis
+    COMPONENT CXX
 )
 install(
     FILES ${CMAKE_CURRENT_BINARY_DIR}/ePhotosynthesis_export.h
     DESTINATION ${EPHOTOSYNTHESIS_INSTALL_INCLUDEDIR}
+    COMPONENT CXX
 )
 # install(
 #      DIRECTORY param
@@ -504,15 +508,56 @@ endif()
 ##########
 
 if(BUILD_PYTHON)
-    find_package(PythonInterp 3)
-    find_package(PythonLibs 3 REQUIRED)
-    add_library(pyPhotosynthesis MODULE ${SOURCES})
-    include_directories(${PYTHON_INCLUDE_DIRS})
-    find_package(Boost 1.36.0 REQUIRED COMPONENTS regex python${PYTHON_VERSION_MAJOR}${PYTHON_VERSION_MINOR})
-    target_link_libraries(pyPhotosynthesis boost_python${PYTHON_VERSION_MAJOR}${PYTHON_VERSION_MINOR})
-    target_link_libraries(pyPhotosynthesis boost_regex)
-    target_link_libraries(pyPhotosynthesis ${PYTHON_LIBRARIES})
-    target_link_libraries(pyPhotosynthesis ${SUNDIALS_LIBRARIES})
-    target_compile_options(pyPhotosynthesis PRIVATE -DBUILD_PYTHON)
-    set_target_properties(pyPhotosynthesis PROPERTIES PREFIX "")
+    set(PYTHON_PACKAGE_NAME ePhotosynthesis)
+    set(PYTHON_LIBRARY_NAME pyPhotosynthesis)
+    find_package(
+        Python REQUIRED COMPONENTS
+        Interpreter Development.Module
+    )
+    find_package(
+        Boost 1.36.0 REQUIRED COMPONENTS
+        regex python${Python_VERSION_MAJOR}${Python_VERSION_MINOR}
+    )
+    add_library(${PYTHON_LIBRARY_NAME} MODULE ${SOURCES})
+    target_link_libraries(
+        ${PYTHON_LIBRARY_NAME}
+        ${Boost_LIBRARIES}
+        Python::Module
+        ${SUNDIALS_LIBRARIES}
+    )
+    target_include_directories(
+        ${PYTHON_LIBRARY_NAME} PUBLIC
+        ${Boost_INCLUDE_DIRS}
+        ${Python_INCLUDE_DIRS}
+        ${SUNDIALS_INCLUDE_DIRS}
+        $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>
+        $<BUILD_INTERFACE:${CMAKE_CURRENT_BINARY_DIR}>
+        $<INSTALL_INTERFACE:include/ePhotosynthesis>
+    )
+    target_compile_options(
+        ${PYTHON_LIBRARY_NAME} PRIVATE -DBUILD_PYTHON
+        -DPYTHON_LIBRARY_NAME=${PYTHON_LIBRARY_NAME}
+    )
+    set_target_properties(${PYTHON_LIBRARY_NAME} PROPERTIES PREFIX "")
+
+    # Tests (requires the Python package actually be installed)
+    add_test(NAME python_tests
+             COMMAND pytest -svx ${PROJECT_SOURCE_DIR}/tests/python)
+
+    # Installation
+    cmake_path(
+        APPEND Python_SITEARCH ${PYTHON_PACKAGE_NAME}
+        OUTPUT_VARIABLE EPHOTOSYNTHESIS_INSTALL_PYTHONDIR
+    )
+    install(
+        TARGETS ${PYTHON_LIBRARY_NAME}
+        DESTINATION "${EPHOTOSYNTHESIS_INSTALL_PYTHONDIR}"
+        COMPONENT Python
+    )
+    configure_file(cmake/__init__.py.in __init__.py @ONLY)
+    install(
+        FILES ${CMAKE_CURRENT_BINARY_DIR}/__init__.py
+        DESTINATION "${EPHOTOSYNTHESIS_INSTALL_PYTHONDIR}"
+        COMPONENT Python
+    )
 endif()
diff --git a/bin/ePhotosynthesis.cpp b/bin/ePhotosynthesis.cpp
@@ -55,29 +55,6 @@ using namespace ePhotosynthesis;
     if (result.count(#x) == 0 && inputs.count(#x) > 0)  \
         x = inputs.at(#x);
 
-#define displayInputVar(fmt, src, val)				\
-    printf("Input variable \"%s\" = " #fmt "\n", #src, val)
-
-#define assignInputVarD(src, dst)                                       \
-    if (inputs.count(#src) > 0) {                                       \
-        theVars-> dst = static_cast<double>(stof(inputs.at(#src), nullptr)); \
-        displayInputVar(%lf, src, theVars-> dst);                       \
-    } else								\
-        printf("Input variable \"%s\" not set\n", #src)
-#define assignInputVarI(src, dst)                                       \
-    if (inputs.count(#src) > 0) {                                       \
-        theVars-> dst = stoi(inputs.at(#src), nullptr);			\
-        displayInputVar(%d, src, theVars-> dst);                        \
-    } else								\
-        printf("Input variable \"%s\" not set\n", #src)
-
-#define setInputVarB(src, mod, dst)                                     \
-    if (inputs.count(#src) > 0) {                                       \
-        modules::mod::set ## dst ((bool)(stoi(inputs.at(#src), nullptr))); \
-        displayInputVar(%d, src, stoi(inputs.at(#src), nullptr));       \
-    } else								\
-        printf("Input variable \"%s\" not set\n", #src)
-
 
 enum DriverType {
     None,
@@ -125,9 +102,9 @@ int main(int argc, const char* argv[]) {
                 ("c,c3", "Use the C3 model, automatically set to true for EPS driver", cxxopts::value<bool>(useC3)->default_value("false"))
 	        ("rubiscomethod",
                  "The method to use for rubisco calculations. Choices are:\n"
-                 "1 - (default) Use enzyme concentration for\n"
+                 "1 - Use enzyme concentration for\n"
                  "    calculation\n"
-                 "2 - Use the michaelis menton and enzyme\n"
+                 "2 - (default) Use the michaelis menton and enzyme\n"
                  "    concentration together for calculation",
                  cxxopts::value<int>(RUBISCOMETHOD))
                 ("t,abstol", "Absolute tolerance for calculations", cxxopts::value<double>(abstol)->default_value("1e-5"))
@@ -171,68 +148,29 @@ int main(int argc, const char* argv[]) {
         if (driverChoice == EPS)
             useC3 = true;
 
+        Variables *theVars = new Variables();
+        // Ensure that command line argument takes precedence
+        if (result.count("Tp") != 0) {
+            theVars->Tp = Tp;
+            inputs["Tp_SET"] = std::to_string(Tp);
+        }
         if (fileProvided(evn, evn))
-          readFile(evn, inputs);
+            theVars->readParam(evn, inputs);
         if (fileProvided(atpcost, atpcost))
-          readFile(atpcost, inputs);
-        Variables *theVars = new Variables();
+            theVars->readParam(atpcost, inputs);
         if (fileProvided(enzyme, enzymeFile)) {
-            std::cerr << "ENZYME DATA PROVIDED" << std::endl;
-            readFile(enzymeFile, theVars->EnzymeAct, true);
+            theVars->readEnzymeAct(enzymeFile);
         } else {
             if (useC3)
                 throw std::runtime_error("Enzyme data required if --c3 set (automatically true for EPS driver)");
         }
 
-        assignInputVarD(CO2, CO2_in);
-        assignInputVarD(PAR, TestLi);
-        assignInputVarD(ATPCost, TestATPCost);
-        if (result.count("Tp") == 0) {
-            assignInputVarD(WeatherTemperature, Tp);
-            Tp = theVars->Tp;
-        }
-        assignInputVarI(GRNC, GRNC);
-        setInputVarB(SucPath, CM, TestSucPath);
-
         // Read the GRN data and assign it into the correct positions
         // based on the expected order
-        if (fileProvided(grn, grnFile)) {
-            std::cerr << "GRN DATA PROVIDED" << std::endl;
-            std::map<std::string, double> glymaID_map;
-            readFile(grnFile, glymaID_map);
-            double pcfactor = 1.0 / 0.973;
-            if (inputs.count("ProteinTotalRatio") > 0)
-                pcfactor = 1.0 / static_cast<double>(stof(inputs.at("ProteinTotalRatio"), nullptr));
-            size_t i = 0;
-            for (auto it = glymaID_order.begin(); it != glymaID_order.end(); it ++, i++) {
-                double iVfactor = pcfactor;
-                try {
-                    if ((i >= 33) && (theVars->GRNC == 0))
-                        iVfactor = 1.0;
-                    else
-                        iVfactor = pcfactor * glymaID_map.at(*it);
-                } catch (const std::out_of_range&) {
-                    // Do nothing
-                    printf("GlymaID \"%s\" not present.\n", it->c_str());
-                }
-                if (i < 33) {
-                    theVars->VfactorCp[i] = iVfactor;
-                } else if (i == 33) {
-                    modules::BF::setcATPsyn(iVfactor);
-                } else if (i == 34) {
-                    modules::BF::setCPSi(iVfactor);
-                } else if (i == 35) {
-                    modules::FI::setcpsii(iVfactor);
-                } else if (i == 36) {
-                    modules::BF::setcNADPHsyn(iVfactor);
-                } else {
-                    printf("More GlymaIDs than expected.\n");
-                    exit(EXIT_FAILURE);
-                }
-            }
-        }
+        if (fileProvided(grn, grnFile))
+            theVars->readGRN(grnFile);
 
-        theVars->Tp = Tp;
+        Tp = theVars->Tp;
         theVars->record = record;
         theVars->useC3 = useC3;
         theVars->RUBISCOMETHOD = RUBISCOMETHOD;

diff --git a/cmake/__init__.py.in b/cmake/__init__.py.in
@@ -0,0 +1 @@
+from .pyPhotosynthesis import Variables, modules, drivers
diff --git a/environment.yml b/environment.yml
@@ -10,4 +10,5 @@ dependencies:
   - pytest
   - python
   - sundials
+  - numpy
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		from .pyPhotosynthesis import Variables, modules, drivers
-Original file line number
+Diff line change
@@ Expand Up / @@ -10,4 +10,5 @@ dependencies: @@
       - pytest
       - python
       - sundials
+      - numpy