add prefix to nested systems to avoid name conflicts (#1574)

* add connector path to mapped_crefs
* add component connectors to mapped_cref to resolve connections
* use full cref to map
* use full cref to map and setValue
* validate_cref and raise error
* use more robust approach to search for mapped crefs
* add connectors_info from componenttable
* use exportName on top level connectors
* check for signals not present in connectors
* allow "-" to valid ident
* set alias name for top level system and subsystem connectors

Author: arun3688

include/OMSimulator/OMSimulator.h

@@ -69,7 +69,8 @@ OMSAPI oms_status_enu_t OMSCALL oms_addResources(const char* cref, const char* p
 OMSAPI oms_status_enu_t OMSCALL oms_addSignalsToResults(const char* cref, const char* regex);
 OMSAPI oms_status_enu_t OMSCALL oms_addStaticValueIndicator(const char* signal, double lower, double upper, double stepSize);
 OMSAPI oms_status_enu_t OMSCALL oms_addSubModel(const char* cref, const char* fmuPath);
-OMSAPI oms_status_enu_t OMSCALL oms_setExportName(const char* cref, const char* exportName); // set export name for a submodel
+OMSAPI oms_status_enu_t OMSCALL oms_setExportName(const char* cref, const char* exportName); // set export name for a submodel, system and connectors
+OMSAPI oms_status_enu_t OMSCALL oms_setAliasName(const char* cref, const char* aliasName); // set alias name for top level system and sub-system connectors
 OMSAPI oms_status_enu_t OMSCALL oms_addSystem(const char* cref, oms_system_enu_t type);
 OMSAPI oms_status_enu_t OMSCALL oms_addTimeIndicator(const char* signal);
 OMSAPI int OMSCALL oms_compareSimulationResults(const char* filenameA, const char* filenameB, const char* varA, const char* varB, double relTol, double absTol);

src/OMSimulatorLib/ComRef.cpp

@@ -37,7 +37,7 @@
 #include <regex>
 #include <cstring>
 
-const std::regex re_ident("^[a-zA-Z][a-zA-Z0-9_]*$");
+const std::regex re_ident("^[a-zA-Z][a-zA-Z0-9_-]*$");
 
 oms::ComRef::ComRef()
 {

src/OMSimulatorLib/Connector.h

@@ -71,9 +71,11 @@ namespace oms
     void setOwner(const oms::ComRef& owner);
     void setGeometry(const oms::ssd::ConnectorGeometry* newGeometry);
     oms_status_enu_t setExportName(const std::string & exportName) { this->exportName = exportName; return oms_status_ok;};
+    oms_status_enu_t setAliasName(const std::string & aliasName) { this->aliasName = aliasName; return oms_status_ok;}; // to be used in the result file to avoid name conflicts
     oms_status_enu_t setNumericType(oms_signal_numeric_type_enu_t numericType) { this->numericType = numericType; return oms_status_ok; }
     oms_signal_numeric_type_enu_t getNumericType() const { return this->numericType; }
     std::string getExportName() const { return this->exportName; }
+    std::string getAliasName() const { return this->aliasName; }
 
     std::map<std::string, std::map<std::string, std::string>> connectorUnits;  ///< single entry map which contains unit as key and BaseUnits as value for a connector
     std::map<std::string, std::string> enumerationName;  ///< single entry map which contains connector name as key and enumerationName as value for a connector of type ssc:Enumeration
@@ -99,7 +101,8 @@ namespace oms
   private:
     friend bool operator==(const Connector& v1, const Connector& v2);
     friend bool operator!=(const Connector& v1, const Connector& v2);
-    std::string exportName;  ///< name to be used in result file
+    std::string exportName = "";  ///< name to be used in result file
+    std::string aliasName = ""; ///< connector name defined in the ssp file
     oms_signal_numeric_type_enu_t numericType = oms_signal_numeric_type_none;  ///< numeric type for the connector for FMI 3.0
   };
 

src/OMSimulatorLib/OMSimulator.cpp

@@ -796,6 +796,23 @@ oms_status_enu_t oms_setExportName(const char* cref, const char* exportName)
   return system->setExportName(tail, exportName);
 }
 
+oms_status_enu_t oms_setAliasName(const char* cref, const char* aliasName)
+{
+  oms::ComRef tail(cref);
+  oms::ComRef front = tail.pop_front();
+
+  oms::Model* model = oms::Scope::GetInstance().getModel(front);
+  if (!model)
+    return logError_ModelNotInScope(front);
+
+  front = tail.pop_front();
+  oms::System* system = model->getSystem(front);
+  if (!system)
+    return logError_SystemNotInModel(model->getCref(), front);
+
+  return system->setAliasName(tail, aliasName);
+}
+
 oms_status_enu_t oms_activateVariant(const char* crefA, const char* crefB)
 {
   oms::ComRef tail(crefA);

src/OMSimulatorLib/System.cpp

@@ -360,6 +360,20 @@ oms_status_enu_t oms::System::setExportName(const oms::ComRef& cref, const std::
   return oms_status_ok;
 }
 
+oms_status_enu_t oms::System::setAliasName(const oms::ComRef& cref, const std::string& aliasName)
+{
+  auto component = getComponent(cref);
+  if (component)
+    return component->setExportName(aliasName);
+
+    // set export name for top level system and subsystem connectors
+  auto connector = getConnector(cref);
+  if (connector)
+    return connector->setAliasName(aliasName);
+
+  return oms_status_ok;
+}
+
 std::string oms::System::getFmiVersion(const std::string& path)
 {
   // unpack the modelDescription.xml in memory to detect the fmiVersion
@@ -2397,13 +2411,6 @@ oms_status_enu_t oms::System::registerSignalsForResultFile(ResultWriter& resultF
 
   resultFileMapping.clear();
 
-  // check for exportName, to be used in result file to map the variable to the correct signal in ssp
-  std::string name;
-  if (!exportName.empty())
-    name = this->exportName;
-  else
-    name = getFullCref();
-
   for (unsigned int i=0; i<connectors.size(); ++i)
   {
     if (!connectors[i])
@@ -2416,23 +2423,23 @@ oms_status_enu_t oms::System::registerSignalsForResultFile(ResultWriter& resultF
     // check for exportName, to be used in result file to map the variable to the correct signal in ssp
     std::string name;
     if (!connector->getExportName().empty())
-      name = connector->getExportName().c_str();
+      name = std::string(ComRef(connector->getExportName()) + connector->getAliasName());
     else
-      name = getFullCref().c_str();
+      name = std::string(getFullCref() + connector->getName());
 
     if (oms_signal_type_real == connector->getType())
     {
-      unsigned int ID = resultFile.addSignal(std::string(ComRef(name) + connector->getName()), "connector", SignalType_REAL);
+      unsigned int ID = resultFile.addSignal(name, "connector", SignalType_REAL);
       resultFileMapping[ID] = i;
     }
     else if (oms_signal_type_integer == connector->getType())
     {
-      unsigned int ID = resultFile.addSignal(std::string(ComRef(name) + connector->getName()), "connector", SignalType_INT);
+      unsigned int ID = resultFile.addSignal(name, "connector", SignalType_INT);
       resultFileMapping[ID] = i;
     }
     else if (oms_signal_type_boolean == connector->getType())
     {
-      unsigned int ID = resultFile.addSignal(std::string(ComRef(name) + connector->getName()), "connector", SignalType_BOOL);
+      unsigned int ID = resultFile.addSignal(name, "connector", SignalType_BOOL);
       resultFileMapping[ID] = i;
     }
   }

src/OMSimulatorLib/System.h

@@ -88,6 +88,7 @@ namespace oms
     oms_status_enu_t addSubSystem(const ComRef& cref, oms_system_enu_t type);
     oms_status_enu_t addSubModel(const ComRef& cref, const std::string& fmuPath);
     oms_status_enu_t setExportName(const ComRef& cref, const std::string& exportName);
+    oms_status_enu_t setAliasName(const ComRef& cref, const std::string& aliasName);
     oms_status_enu_t replaceSubModel(const ComRef& cref, const std::string& fmuPath, bool dryRun, int& warningCount);
     bool isValidScalarVariable(Component* referenceComponent, Component* replacingComponent, Connection* connection, const ComRef& crefA, const ComRef& crefB, const ComRef& signalName, const std::string& path, std::vector<std::string>& warningList);
     bool validCref(const ComRef& cref);

src/OMSimulatorLib/SystemWC.cpp

@@ -939,23 +939,23 @@ oms_status_enu_t oms::SystemWC::registerSignalsForResultFile(ResultWriter& resul
     // check for exportName, to be used in result file to map the variable to the correct signal in ssp
     std::string name;
     if (!connector->getExportName().empty())
-      name = connector->getExportName().c_str();
+      name = std::string(ComRef(connector->getExportName()) + connector->getAliasName());
     else
-      name = getFullCref().c_str();
+      name = std::string(ComRef(getFullCref()) + connector->getName());
 
-  if (oms_signal_type_real == connector->getType())
-  {
-    unsigned int ID = resultFile.addSignal(std::string(ComRef(name) + connector->getName()), "connector", SignalType_REAL);
-    resultFileMapping[ID] = i;
-  }
+    if (oms_signal_type_real == connector->getType())
+    {
+      unsigned int ID = resultFile.addSignal(name, "connector", SignalType_REAL);
+      resultFileMapping[ID] = i;
+    }
     else if (oms_signal_type_integer == connector->getType())
     {
-      unsigned int ID = resultFile.addSignal(std::string(ComRef(name) + connector->getName()), "connector", SignalType_INT);
+      unsigned int ID = resultFile.addSignal(name, "connector", SignalType_INT);
       resultFileMapping[ID] = i;
     }
     else if (oms_signal_type_boolean == connector->getType())
     {
-      unsigned int ID = resultFile.addSignal(std::string(ComRef(name) + connector->getName()), "connector", SignalType_BOOL);
+      unsigned int ID = resultFile.addSignal(name, "connector", SignalType_BOOL);
       resultFileMapping[ID] = i;
     }
   }

src/OMSimulatorPython/capi.py

@@ -114,6 +114,8 @@ def __init__(self):
     self.obj.oms_setTempDirectory.restype = ctypes.c_int
     self.obj.oms_setExportName.argtypes = [ctypes.c_char_p, ctypes.c_char_p]
     self.obj.oms_setExportName.restype = ctypes.c_int
+    self.obj.oms_setAliasName.argtypes = [ctypes.c_char_p, ctypes.c_char_p]
+    self.obj.oms_setAliasName.restype = ctypes.c_int
     self.obj.oms_setBoolean.argtypes = [ctypes.c_char_p, ctypes.c_bool]
     self.obj.oms_setBoolean.restype = ctypes.c_int
     self.obj.oms_setInteger.argtypes = [ctypes.c_char_p, ctypes.c_int]
@@ -285,6 +287,12 @@ def setExportName(self, cref, exportName):
     status = self.obj.oms_setExportName(cref.encode(), exportName.encode())
     return Status(status)
 
+  def setAliasName(self, cref, aliasName):
+    '''Set the export name for top level system and sub-system connectors
+    This is used to specify the name under which the model will be exported.'''
+    status = self.obj.oms_setAliasName(cref.encode(), aliasName.encode())
+    return Status(status)
+
   def setReal(self, cref, value):
     '''Set a real value for a model or system.'''
     status = self.obj.oms_setReal(cref.encode(), value)

src/OMSimulatorPython/instantiated_model.py

@@ -110,7 +110,12 @@ def __init__(self, json_description, system: System, resources: dict):
       listofsystems = []
       ## add components
       for comp in unit["components"]:
-        comp_path = ".".join([solver_path] + [comp["name"][-1]])
+        if len(comp["name"]) <= 2:
+          comp_path = ".".join([solver_path] + [comp["name"][-1]])
+        else:
+          ## add prefix to nested systems to avoid name conflicts while flattening the system during instantiation, e.g. sub-system1_Add1, sub-system1_Gain1
+          comp_name = f"{comp['name'][-2]}_{comp['name'][-1]}"
+          comp_path = ".".join([solver_path] + [comp_name])
         currentSystem = ".".join(comp["name"][:-1])
         if currentSystem not in listofsystems:
           listofsystems.append(currentSystem)
@@ -124,6 +129,10 @@ def __init__(self, json_description, system: System, resources: dict):
         ## parse connector geometry for the component if exist and set it to capi after adding the component, this is needed for proper mapping of connector geometry
         if "connectors" in comp:
           for connector in comp["connectors"]:
+            comp_name = ".".join(comp["name"])
+            connector_name =".".join([comp_name]+[connector["name"]])
+            if not connector_name in self.mappedCrefs:
+              self.mappedCrefs[connector_name] = f"{comp_path}.{connector['name']}"
             if "geometry" in connector:
               connector_path = ".".join([comp_path, connector["name"]])
               geometry = connector["geometry"]
@@ -168,10 +177,16 @@ def __init__(self, json_description, system: System, resources: dict):
 
       ## add connections
       for connection in unit["connections"]:
-        start_element = ".".join(connection['start element'])
-        end_element = ".".join(connection['end element'])
-        start = self.mappedCrefs[start_element] + f".{connection['start connector']}"
-        end = self.mappedCrefs[end_element] + f".{connection['end connector']}"
+        start_element = ".".join(connection['start element'] + [connection['start connector']])
+        end_element = ".".join(connection['end element'] + [connection['end connector']])
+
+        if start_element not in self.mappedCrefs:
+          raise KeyError(f"No mapping found for {start_element}")
+        if end_element not in self.mappedCrefs:
+          raise KeyError(f"No mapping found for {end_element}")
+
+        start = self.mappedCrefs[start_element]
+        end = self.mappedCrefs[end_element]
         self.apiCall.append(f'oms_addConnection("{start}", "{end}")')
         status = Capi.addConnection(start, end)
         if status != Status.ok:
@@ -265,6 +280,27 @@ def validatePath(self, mapped_prefix : list, suffix : list):
 
     return ".".join(mapped_prefix + result)
 
+  def validate_cref(self, systemName: str, suffix: str) -> str:
+    ## remove common path from suffix
+    ## e.g. systemName = model.root.solver2.gain1
+    ##      suffix = gain1.R1.T
+    ##      result = model.root.solver2.gain1.R1.T
+
+    a_parts = systemName.split(".")
+    b_parts = suffix.split(".")
+
+    # detect overlap
+    overlap = 0
+    for i in range(1, min(len(a_parts), len(b_parts)) + 1):
+      if a_parts[-i:] == b_parts[:i]:
+        overlap = i
+    cref = ".".join(a_parts + b_parts[overlap:])
+    if cref in self.mappedCrefs:
+      return self.mappedCrefs[cref]
+    else:
+      ## it is possible some variables are not listed in connectors but it is still valid signal
+      return self.map_cref(systemName, suffix)
+
   def setStartValues(self, value: Values, systemName: str, ssm: SSM | None):
     if value.empty():
       return
@@ -280,13 +316,15 @@ def setStartValues(self, value: Values, systemName: str, ssm: SSM | None):
           for entry in targets:
             target = entry["target"]
             linearTransformation = entry["linearTransformation"]
-            value_path = self.map_cref(systemName, str(target))
+            cref = f"{systemName}.{str(target)}"
+            value_path = self.validate_cref(systemName, str(target))
             if linearTransformation:
               source_value = source_value * float(linearTransformation.factor) + float(linearTransformation.offset)
             self.apply_start_value(value_path, source_value, type)
     else:
       for key, (source_value, type, _, _) in value.start_values.items():
-        value_path = self.map_cref(systemName, str(key))
+        cref = f"{systemName}.{str(key)}"
+        value_path = self.validate_cref(systemName, str(key))
         self.apply_start_value(value_path, source_value, type)
 
   def apply_start_value(self, value_path:str, value, type):
@@ -314,8 +352,14 @@ def apply_start_value(self, value_path:str, value, type):
 
   def _addConnector(self, connectors, systemName):
     for connector in connectors:
-      name = [systemName] + [str(connector.name)]
-      connector_path = ".".join([self.mappedCrefs[systemName], str(connector.name)])
+      connector_name =".".join([systemName]+[str(connector.name)])
+      system_name = systemName.split(".")
+      if len(system_name) == 1:
+        connector_path = ".".join([self.mappedCrefs[systemName], str(connector.name)])
+      else:
+        ## add prefix to nested systems to avoid name conflicts while flattening the system during instantiation, e.g. sub-system1_input1, sub-system1_param1
+        connector_name_prefix = f"{system_name[-1]}_{str(connector.name)}"  # replace '-' with '_' for sub-system name in connector path
+        connector_path = ".".join([self.mappedCrefs[systemName], connector_name_prefix])
       self.apiCall.append(f'oms_addConnector("{connector_path}", "{connector.causality}", {connector.signal_type})')
       status = Capi.addConnector(connector_path, connector.causality.value, connector.c_signal_type.value)
       if status != Status.ok:
@@ -336,11 +380,14 @@ def _addConnector(self, connectors, systemName):
           raise RuntimeError(f"Failed to set connector geometry for {connector_path}: {status}")
 
       export_name = systemName
-      if not export_name in self.mappedCrefs:
-        self.mappedCrefs[export_name] = connector_path
+      if not connector_name in self.mappedCrefs:
+        self.mappedCrefs[connector_name] = connector_path
       status = Capi.setExportName(connector_path, export_name)  # Set export name if provided
       if status != Status.ok:
         raise RuntimeError(f"Failed to set export name: {status}")
+      status = Capi.setAliasName(connector_path, str(connector.name))  # Set alias name for connector
+      if status != Status.ok:
+        raise RuntimeError(f"Failed to set alias name: {status}")
 
   def addConnectorFromElements(self, elements, currentSystem):
     ## add connectors mapped with currentSystem
@@ -359,11 +406,18 @@ def dumpApiCalls(self):
 
   def setValue(self, cref: CRef, value):
     """Sets a value for a specific CRef in the model."""
-    name = ".".join(cref.names[:-1])
-    if name not in self.mappedCrefs:
-      raise KeyError(f"Missing required key: '{name}'")
+    name = ".".join(cref.names)
 
-    value_path = ".".join([self.mappedCrefs[name], cref.names[-1]])
+    ## check for top level connectors with alias names first
+    ## e.g model.root.sub_system_input which will be in mapped crefs
+    if name in self.mappedCrefs:
+      value_path = self.mappedCrefs[name]
+    else:
+      ## check for other crefs by splitting with suffixes
+      name = ".".join(cref.names[:-1])
+      if name not in self.mappedCrefs:
+        raise KeyError(f"Missing required key: '{cref.names}'")
+      value_path = ".".join([self.mappedCrefs[name], cref.names[-1]])
 
     # Determine the variable type
     type, status = Capi.getVariableType(value_path)
@@ -387,35 +441,41 @@ def setValue(self, cref: CRef, value):
         raise TypeError(f"Unsupported type: {type}")
 
   def _setReal(self, mapped_cref: str, value: float):
-    self.apiCall.append(f'oms_setReal("{mapped_cref}, {value})')
+    self.apiCall.append(f'oms_setReal("{mapped_cref}", {value})')
     status = Capi.setReal(mapped_cref, value) # Get the value from the CAPI
     if status != Status.ok:
       raise RuntimeError(f"Failed to set value for {mapped_cref}: {status}")
 
   def _setInteger(self, mapped_cref: str, value: int):
-    self.apiCall.append(f'oms_setInteger("{mapped_cref}, {value})')
+    self.apiCall.append(f'oms_setInteger("{mapped_cref}", {value})')
     status = Capi.setInteger(mapped_cref, value) # Get the value from the CAPI
     if status != Status.ok:
       raise RuntimeError(f"Failed to set value for {mapped_cref}: {status}")
 
   def _setBoolean(self, mapped_cref: str, value: bool):
-    self.apiCall.append(f'oms_setBoolean("{mapped_cref}, {value})')
+    self.apiCall.append(f'oms_setBoolean("{mapped_cref}", {value})')
     status = Capi.setBoolean(mapped_cref, value) # Get the value from the CAPI
     if status != Status.ok:
       raise RuntimeError(f"Failed to set value for {mapped_cref}: {status}")
 
   def _setString(self, mapped_cref: str, value: str):
-    self.apiCall.append(f'oms_setString("{mapped_cref}, {value})')
+    self.apiCall.append(f'oms_setString("{mapped_cref}", {value})')
     status = Capi.setString(mapped_cref, value) # Get the value from the CAPI
     if status != Status.ok:
       raise RuntimeError(f"Failed to set value for {mapped_cref}: {status}")
 
   def getValue(self, cref: CRef):
-    name = ".".join(cref.names[:-1])
-    if name not in self.mappedCrefs:
-      raise KeyError(f"Missing required key: '{name}'")
-
-    value_path = ".".join([self.mappedCrefs[name], cref.names[-1]])
+    name = ".".join(cref.names)
+    ## check for top level connectors with alias names first
+    ## e.g model.root.sub_system_input which will be in mapped crefs
+    if name in self.mappedCrefs:
+      value_path = self.mappedCrefs[name]
+    else:
+      ## check for other crefs by splitting with suffixes
+      name = ".".join(cref.names[:-1])
+      if name not in self.mappedCrefs:
+        raise KeyError(f"Missing required key: '{cref.names}'")
+      value_path = ".".join([self.mappedCrefs[name], cref.names[-1]])
 
     # Determine the variable type
     type, status = Capi.getVariableType(value_path)