Add test case for replaceFD issue #299

src/bindings/csharp/test/sbml/TestSBMLTransforms.cs

@@ -148,7 +148,7 @@ public void test_SBMLTransforms_replaceFD()
       {
       }
       m = d.getModel();
-      assertTrue( m.getNumFunctionDefinitions() == 2 );
+      assertTrue( m.getNumFunctionDefinitions() == 3 );
       d.expandFunctionDefinitions();
       assertTrue( d.getModel().getNumFunctionDefinitions() == 0 );
       ast = d.getModel().getReaction(0).getKineticLaw().getMath();

src/bindings/java/test/org/sbml/libsbml/test/sbml/TestSBMLTransforms.java

@@ -146,7 +146,7 @@ public void test_SBMLTransforms_replaceFD()
     {
     }
     m = d.getModel();
-    assertTrue( m.getNumFunctionDefinitions() == 2 );
+    assertTrue( m.getNumFunctionDefinitions() == 3 );
     d.expandFunctionDefinitions();
     assertTrue( d.getModel().getNumFunctionDefinitions() == 0 );
     ast = d.getModel().getReaction(0).getKineticLaw().getMath();

src/sbml/SBMLTransforms.cpp

@@ -146,9 +146,52 @@ SBMLTransforms::replaceBvars(ASTNode * node, const FunctionDefinition *fd)
     noBvars = fd->getMath()->getNumBvars();
     fdMath = *fd->getBody();
 
-    unsigned int nodeCount = 0;
+    // get names of all bvars
+    std::vector<std::string> allBVars; 
+    for (unsigned int i = 0; i < noBvars; ++i)
+      allBVars.push_back(fd->getArgument(i)->getName());
+
+		// get all names in the node
+		List* names = node->getListOfNodes((ASTNodePredicate)ASTNode_isName);
+
+		// convert to std::vector
+    std::vector<std::string> currentChildNames;
+		for (unsigned int j = 0 ; j < names->getSize(); ++j)
+		{
+			ASTNode* name = (ASTNode*)names->get(j);
+			currentChildNames.push_back(name->getName());
+		}
+		delete names;
+
+    // establish order in which to replace bvars, we want to ensure that 
+		// no bvars are replaced before they are used
+		std::vector<unsigned int> replaceOrder;
     for (unsigned int i = 0; i < noBvars; ++i)
     {
+			std::string currentArg = fd->getArgument(i)->getName();
+      bool added = false;
+			for (unsigned int j = 0; j < currentChildNames.size(); ++j)
+			{
+				if (currentArg == currentChildNames[j])
+				{
+					replaceOrder.insert(replaceOrder.begin(), i);					
+          added = true;
+					break;
+				}
+			}
+	    if (!added)
+	    {
+		    replaceOrder.push_back(i);
+	    }
+    }
+
+    unsigned int nodeCount = 0;
+
+		// now replace in the order established above
+    std::vector<unsigned int>::iterator it = replaceOrder.begin();
+    for (; it != replaceOrder.end(); ++it)
+    {
+      unsigned int i = *it;
       if (nodeCount < node->getNumChildren())
       {
         fdMath.replaceArgument(fd->getArgument(i)->getName(), 

src/sbml/test/TestSBMLTransforms.cpp

@@ -73,15 +73,15 @@ START_TEST (test_SBMLTransforms_replaceFD)
 
 
   d = reader.readSBML(filename);
-
+  
   if (d == NULL)
   {
     fail("readSBML(\"multiple-functions.xml\") returned a NULL pointer.");
   }
 
   m = d->getModel();
 
-  fail_unless( m->getNumFunctionDefinitions() == 2 );
+  fail_unless( m->getNumFunctionDefinitions() == 3 );
 
   /* one function definition */
   ast = *m->getReaction(2)->getKineticLaw()->getMath();
@@ -97,13 +97,35 @@ START_TEST (test_SBMLTransforms_replaceFD)
   fail_unless (!strcmp(math, "S1 * p * compartmentOne / t"), NULL);
   safe_free(math);
 
+  /* https://github.com/sbmlteam/libsbml/issues/299 */
+  /* fd: f_relabelled(p, S1) = p * S1 */
+  /* ast: f_relabelled(S1, p) * compartmentOne / t */
+  /* ast after replaceFD: p * S1 * compartmentOne / t */
+
+  // use initial assignment instead of reaction, because the reaction
+  // got flagged as having invalid units
+  ast = *m->getInitialAssignment(0)->getMath();
+
+  math = SBML_formulaToString(&ast);
+  fail_unless (!strcmp(math, "f_relabelled(S1, p) * compartmentOne / t"), NULL);
+  safe_free(math);
+
+  fd = m->getFunctionDefinition(2);
+  SBMLTransforms::replaceFD(&ast, fd);
+
+  math = SBML_formulaToString(&ast);
+  fail_unless (!strcmp(math, "p * S1 * compartmentOne / t"), NULL);
+  safe_free(math);
+
   /* one function definition - nested */
   ast = *m->getReaction(1)->getKineticLaw()->getMath();
 
   math = SBML_formulaToString(&ast);
   fail_unless (!strcmp(math, "f(f(S1, p), compartmentOne) / t"), NULL);
   safe_free(math);
 
+  // need to get function definition for 'f' back
+  fd = m->getFunctionDefinition(0);
   SBMLTransforms::replaceFD(&ast, fd);
 
   math = SBML_formulaToString(&ast);

src/sbml/test/test-data/multiple-functions.xml

@@ -1,5 +1,5 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<sbml xmlns="http://www.sbml.org/sbml/level2" level="2" version="1">
+<sbml xmlns="http://www.sbml.org/sbml/level2/version4" level="2" version="4">
   <model>
     <listOfFunctionDefinitions>
       <functionDefinition id="f">
@@ -36,6 +36,23 @@
           </lambda>
         </math>
       </functionDefinition>
+      <functionDefinition id="f_relabelled">
+        <math xmlns="http://www.w3.org/1998/Math/MathML">
+          <lambda>
+            <bvar>
+              <ci> p </ci>
+            </bvar>
+            <bvar>
+              <ci> S1 </ci>
+            </bvar>
+            <apply>
+              <times/>
+              <ci> p </ci>
+              <ci> S1 </ci>
+            </apply>
+          </lambda>
+        </math>
+      </functionDefinition>
     </listOfFunctionDefinitions>
     <listOfCompartments>
       <compartment id="compartmentOne" size="1"/>
@@ -47,7 +64,27 @@
     <listOfParameters>
       <parameter id="t" value="1" units="second"/>
       <parameter id="p" value="1" units="dimensionless"/>
+      <parameter id="p_target" value="1" />
     </listOfParameters>
+    <listOfInitialAssignments>
+      <initialAssignment symbol="p_target">
+        <math xmlns="http://www.w3.org/1998/Math/MathML">
+          <apply>
+            <divide/>
+            <apply>
+              <times/>
+              <apply>
+                <ci> f_relabelled </ci>
+                <ci> S1 </ci>
+                <ci> p </ci>
+              </apply>
+              <ci> compartmentOne </ci>
+            </apply>
+            <ci> t </ci>
+          </apply>
+        </math>
+      </initialAssignment>
+    </listOfInitialAssignments>
     <listOfReactions>
       <reaction id="reaction_1" reversible="false" fast="false">
         <listOfReactants>
@@ -124,6 +161,31 @@
           </math>
         </kineticLaw>
       </reaction>
+      <!-- <reaction id="reaction_3_using_f_relabelled" reversible="false" fast="false">
+        <listOfReactants>
+          <speciesReference species="S1"/>
+        </listOfReactants>
+        <listOfProducts>
+          <speciesReference species="S2"/>
+        </listOfProducts>
+        <kineticLaw>
+          <math xmlns="http://www.w3.org/1998/Math/MathML">
+            <apply>
+              <divide/>
+              <apply>
+                <times/>
+                <apply>
+                  <ci> f_relabelled </ci>
+                  <ci> S1 </ci>
+                  <ci> p </ci>
+                </apply>
+                <ci> compartmentOne </ci>
+              </apply>
+              <ci> t </ci>
+            </apply>
+          </math>
+        </kineticLaw>
+      </reaction> -->
     </listOfReactions>
   </model>
 </sbml>