Clean up input file parsers and add system tests for JSON input files (#121)

* Enable direct joint node assocations by using the jointNode options variable when the model is created. This uses joint node associations defined in JSON input files. The legacy serializer behavior is preserved -- instead of using the listed joint node, we use sequential nodes from the node list to correspond to the joints. That's how createAndRunModel used to extract the node associations.

* Additional clean up of options: (1) eliminate direct global population from legacy serializer. The output options are (for now) part of the options struct... until we can house them elsewhere. This frees the options from interactions with any global variables. (2) Updates to the json serializer: actually use node assocations now that the model will utilize them. Also include outputType and vtkOutputType until we move them.

* replace hardcoded requirements on binary location in system tests (test_integration.py) with an option for the binary location passed to pytest. That enables simple running of pytest regardless of the particular location of the binary directory. For example: pytest Tests/SystemTests --relativeExePath="../svOneDSolver_build/bin/OneDSolver"    Also: make the system tests actually execute tests in the temporary directory so that we don't have artifacts generated in whatever directory we run the tests from.

* add system tests for conversion to JSON input files and simulation using JSON input files. Add documentation for the executable usage and running unit and system tests

Author: arsLibera

.github/workflows/test.yml

@@ -15,11 +15,12 @@ jobs:
           make
       - name: install dependencies
         run: |
-          pip install pytest pytest-cov pytest-mock
+          pip install pytest pytest-cov pytest-mock pytest-xdist
           pip install numpy
-      - name: result tests
+      - name: result simulation system tests
         run: |
-          pytest Tests/SystemTests
+          # We're providing the path to the EXE based on the prior build solver step.
+          pytest Tests/SystemTests -n auto --relativeExePath="build_skyline/bin/OneDSolver" 
       - name: result unit tests 
         run: |
           cd build_skyline

.gitignore

@@ -41,3 +41,6 @@ build
 
 # Vscode
 .vscode/
+
+# Python
+venv/

CMakeLists.txt

@@ -1,5 +1,5 @@
 
-CMAKE_MINIMUM_REQUIRED(VERSION 3.11)
+CMAKE_MINIMUM_REQUIRED(VERSION 3.18)
 
 # COMMON SETTINGS
 PROJECT(OneDSolver)
@@ -71,6 +71,17 @@ ELSEIF(sparseSolverType STREQUAL "csparse")
 			"${CMAKE_CURRENT_SOURCE_DIR}/Code/Source/sparse/csparse/*.h")
 ENDIF()
 
+# NLOHMANN JSON FOR SERIALIZATION
+include(FetchContent)
+FetchContent_Declare(
+    nlohmann_json
+    GIT_REPOSITORY https://github.com/nlohmann/json.git
+    GIT_TAG v3.11.2  # You can specify a version or use the latest tag
+)
+FetchContent_MakeAvailable(nlohmann_json)
+# Global include for nlohmann headers
+include_directories(${nlohmann_json_SOURCE_DIR}/single_include)
+
 # COPY DATASET FOLDER
 # FILE(COPY "${CMAKE_CURRENT_SOURCE_DIR}/tests" DESTINATION "${CMAKE_CURRENT_BINARY_DIR}")
 
@@ -111,16 +122,6 @@ if (BUILD_SV_INSTALLER)
     add_subdirectory("${CMAKE_SOURCE_DIR}/Distribution")
 ENDIF()
 
-# NLOHMANN JSON FOR SERIALIZATION
-include(FetchContent)
-FetchContent_Declare(
-    nlohmann_json
-    GIT_REPOSITORY https://github.com/nlohmann/json.git
-    GIT_TAG v3.11.2  # You can specify a version or use the latest tag
-)
-FetchContent_MakeAvailable(nlohmann_json)
-target_include_directories(${PROJECT_NAME} PUBLIC ${nlohmann_json_SOURCE_DIR}/single_include)
-
 # Unit tests and Google Test
 if(ENABLE_UNIT_TEST)
 
@@ -174,13 +175,6 @@ if(ENABLE_UNIT_TEST)
   # handle both use cases.
   target_link_libraries(UnitTestsExecutable gtest gtest_main pthread)
 
-  # We need to include Nlohmann when compiling the unit test executable
-  # A better strategy would be to build up a list of what we're going to
-  # include and reuse it.
-  # Another option: we could simply build the library of cvOneD functionality
-  # and link against that both here and in the main executable.  
-  target_include_directories(UnitTestsExecutable PRIVATE ${nlohmann_json_SOURCE_DIR}/single_include)
-
   # It's likely we'll need to include additional directories for some
   # versions of the unit tests. That can be updated when/if we update
   # the general strategy.

Code/Source/cvOneDModelManager.cxx

@@ -188,7 +188,7 @@ int cvOneDModelManager::CreateNode(char * nodeName,double x,double y,double z){
 }
 
 
-int cvOneDModelManager::CreateJoint(char * jointName,double x,double y,double z,
+int cvOneDModelManager::CreateJoint(const char * jointName,double x,double y,double z,
                                     int numInSegs,int numOutSegs,
                                     int *InSegs,int *OutSegs){
 

Code/Source/cvOneDModelManager.h

@@ -69,7 +69,7 @@ class cvOneDModelManager{
 
 
     // CREATE JOINT
-    int CreateJoint(char* jointName,double x,double y,double z,
+    int CreateJoint(const char* jointName,double x,double y,double z,
                     int numInSegs,int numOutSegs,
                     int *InSegs,int *OutSegs);
 

Code/Source/cvOneDOptions.h

@@ -35,6 +35,7 @@
 #include <string>
 #include <math.h>
 #include <memory>
+#include <optional>
 #include "cvOneDTypes.h"
 #include "cvOneDException.h"
 
@@ -49,21 +50,14 @@ struct options{
 
     // NODE DATA
     cvStringVec nodeName;
-    // Note that these coordinates are also used
-    // for the joints through an implicit mapping
     cvDoubleVec nodeXcoord;
     cvDoubleVec nodeYcoord;
     cvDoubleVec nodeZcoord;
 
     // JOINT DATA
     cvStringVec jointName;
-    
-    // "jointNode" isn't used currently -- but we want
-    // to start integrating it. It can be used to make
-    // the mapping from joint->node explicit. Right
-    // now, the mapping is implicit: each joint is 
-    // associated with the node of the same index in
-    // the list of nodes.
+    // jointNode: name of the associated node that we will use 
+    // to get the XYZ coordinates when creating the joint.
     cvStringVec jointNode; 
 
     cvStringVec jointInletName;
@@ -121,7 +115,12 @@ struct options{
     double convergenceTolerance;
     int    useIV;
     int    useStab;
-    string outputType;
+
+    // These are to preserve legacy behavior and are 
+    // expected to be eventually migrated into a 
+    // post-processing step.
+    string outputType = "TEXT";
+    std::optional<int> vtkOutputType = std::nullopt; 
 };
 
 void validateOptions(options const& opts);

Code/Source/cvOneDOptionsJsonParser.cxx

@@ -206,8 +206,13 @@ void parseSolverOptions(const nlohmann::ordered_json& jsonData, options& opts) t
     opts.useIV = solverOptions.at("useIV").get<int>();
     opts.useStab = solverOptions.at("useStab").get<int>();
 
-    // Doesn't seem like this is used...but we'll provide a default anyway
-    opts.outputType = solverOptions.value("outputType", "None");    
+    // Until this is migrated elsewhere, we'll (optionally) store the solver options.
+    if(solverOptions.contains("outputType")){
+        opts.outputType = solverOptions.at("outputType").get<std::string>();    
+    }
+    if(solverOptions.contains("vtkOutputType")){
+        opts.vtkOutputType = solverOptions.at("vtkOutputType").get<int>();    
+    }
 
 } catch (const std::exception& e) {
     throw std::runtime_error("Error parsing 'solverOptions': " + std::string(e.what()));

Code/Source/cvOneDOptionsJsonSerializer.cxx

@@ -81,29 +81,22 @@ nlohmann::ordered_json serialize_joint_data(const options& opts) {
     nlohmann::ordered_json j = nlohmann::ordered_json::array();
 
     // Check consistency of the joint inlet and outlet vectors
-    check_consistent_size("joints", opts.jointName,
+    check_consistent_size("joints", opts.jointName, opts.jointNode, 
                           opts.jointInletListNames, opts.jointInletListNumber,
                           opts.jointOutletListNames, opts.jointOutletListNumber,
                           opts.jointInletList, opts.jointOutletList);
 
     size_t jointCount = opts.jointName.size();
 
-    // There must be enough nodes for us to make associations. This is 
-    // an implicit assumption made in the current joint creation code.
-    if( opts.nodeName.size() < jointCount ){
-        throw std::runtime_error("The implicit joint-node association requires that there be at least as many nodes as there are joints.");
-    }
-
     for (size_t i = 0; i < jointCount; ++i) {
         nlohmann::ordered_json joint;
 
         // Add JOINT data
-        // We're adding the node based on the index so we
-        // can start incorporating the explicit association
-        // from the implicit one currently used in the code.
+        // Now that we're actually populating the joint node associations
+        // when deserializing the legacy format, we can utilize them here.
         joint["joint"] = {
-            {"id", "J" + std::to_string(i + 1)},
-            {"associatedNode", opts.nodeName.at(i)},
+            {"id", opts.jointName.at(i)},
+            {"associatedNode", opts.jointNode.at(i)},
             {"inletName", opts.jointInletListNames.at(i)},
             {"outletName", opts.jointOutletListNames.at(i)}
         };
@@ -225,7 +218,13 @@ nlohmann::ordered_json serializeSolverOptions(options const& opts) {
     solverOptions["convergenceTolerance"] = opts.convergenceTolerance;
     solverOptions["useIV"] = opts.useIV;
     solverOptions["useStab"] = opts.useStab;
+
+    // For now, we're serializing the output data.
+    // In the future, we'll want to migrate these.
     solverOptions["outputType"] = opts.outputType;
+    if(opts.vtkOutputType){
+        solverOptions["vtkOutputType"] = *opts.vtkOutputType;
+    }
 
     return solverOptions;
 }

Code/Source/cvOneDOptionsLegacySerializer.cxx

@@ -36,7 +36,6 @@
 #include <string.h>
 
 #include "cvOneDUtility.h"
-#include "cvOneDGlobal.h"
 
 namespace cvOneD{
 
@@ -126,10 +125,25 @@ void readOptionsLegacyFormat(string inputFile, options* opts){
         try{
           // Get Joint Name
           opts->jointName.push_back(tokenizedString[1]);
-//          opts->jointNode.push_back(atof(tokenizedString[2].c_str()));
-          opts->jointNode.push_back(tokenizedString[2]);
+
+          // The legacy input files included a "joint node", but it is
+          // never utilized in simulation. The legacy behavior was, instead
+          // to simply associate each joint with the sequentially listed
+          // nodeXcoord, nodeYcoord, and nodeZcoord.
+          //
+          // To preserve this legacy behavior, we're not going to deserialize
+          // the jointNode. Instead, we're going to replace "jointNode" with
+          // the sequential node item name. That's **wrong** -- we should be 
+          // using "jointNode"!
+          // But! it preserves the legacy behavior of the existing legacy 
+          // options files when we refactor the actual utilization of the 
+          // options to use the jointNode.
+          //
+          // The actual population of the jointNode option is done in a 
+          // post-processing step after we finish processing the file.
           opts->jointInletName.push_back(tokenizedString[3]);
           opts->jointOutletName.push_back(tokenizedString[4]);
+
         }catch(...){
           throw cvException(string("ERROR: Invalid JOINT Format. Line " + to_string(lineCount) + "\n").c_str());
         }
@@ -258,22 +272,13 @@ void readOptionsLegacyFormat(string inputFile, options* opts){
         }else if(tokenizedString.size() < 2){
           throw cvException(string("ERROR: Not enough parameters for OUTPUT token. Line " + to_string(lineCount) + "\n").c_str());
         }
-        // Output Type
-        if(upper_string(tokenizedString[1]) == "TEXT"){
-          cvOneDGlobal::outputType = OutputTypeScope::OUTPUT_TEXT;
-        }else if(upper_string(tokenizedString[1]) == "VTK"){
-          cvOneDGlobal::outputType = OutputTypeScope::OUTPUT_VTK;
-        }else if(upper_string(tokenizedString[1]) == "BOTH"){
-          cvOneDGlobal::outputType = OutputTypeScope::OUTPUT_BOTH;
-        }else{
-          throw cvException("ERROR: Invalid OUTPUT Type.\n");
-        }
+
+        // Collect and store the output type data
+        opts->outputType = tokenizedString[1];
         if(tokenizedString.size() > 2){
-          cvOneDGlobal::vtkOutputType = atoi(tokenizedString[2].c_str());
-          if(cvOneDGlobal::vtkOutputType > 1){
-            throw cvException("ERROR: Invalid OUTPUT VTK Type.\n");
-          }
+          opts->vtkOutputType = atoi(tokenizedString[2].c_str());
         }
+
       }else if(upper_string(tokenizedString[0]) == std::string("DATATABLE")){
         // printf("Found Data Table.\n");
         try{
@@ -361,6 +366,17 @@ void readOptionsLegacyFormat(string inputFile, options* opts){
     // Increment Line Count
     lineCount++;
   }
+
+  // Post-process: to preserve legacy behavior, we're
+  // making each joint node the corresponding sequential 
+  // node from the node array. See comment above for additional details.
+  if(opts->jointName.size() > opts->nodeName.size()){
+    throw cvException("ERROR: there must be at least as many nodes as there are joints.\n");
+  }
+  for(size_t k = 0; k < opts->jointName.size(); ++k){
+    opts->jointNode.push_back(opts->nodeName.at(k));
+  }
+
   // Close File
   infile.close();
 }