Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix bug in SBMLTransforms::replaceFD #303

Merged
merged 2 commits into from
Mar 14, 2023
Merged

Fix bug in SBMLTransforms::replaceFD #303

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d7144b7
change operation from mul to minus to show incorrect results for this…
lkeegan Mar 14, 2023
c74c3d7
replace arguments at the same time instead of sequentially
lkeegan Mar 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 7 additions & 52 deletions src/sbml/SBMLTransforms.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,59 +146,14 @@ SBMLTransforms::replaceBvars(ASTNode * node, const FunctionDefinition *fd)
noBvars = fd->getMath()->getNumBvars();
fdMath = *fd->getBody();

// 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(),
node->getChild(nodeCount));
}
++nodeCount;
// get names of bvars and AST node to substitute for each
std::vector<std::string> bVars;
std::vector<ASTNode*> astNodes;
for (unsigned int i = 0; i < noBvars; ++i){
bVars.push_back(fd->getArgument(i)->getName());
astNodes.push_back(node->getChild(i));
}
fdMath.replaceArguments(bVars, astNodes);
(*node) = fdMath;
}

Expand Down
151 changes: 77 additions & 74 deletions src/sbml/math/ASTNode.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2759,100 +2759,103 @@ ASTNode::isWellFormedASTNode() const

/** @endcond */


LIBSBML_EXTERN
void
ASTNode::replaceArgument(const std::string& bvar, ASTNode * arg)
static void copyNode(const ASTNode * source, ASTNode * dest)
{
if (arg == NULL)
return;
else if (getNumChildren() == 0)
{
if (this->isName() && this->getName() == bvar)
if (source == NULL)
{
if (arg->isName())
{
this->setType(arg->getType());
this->setName(arg->getName());
}
else if (arg->isReal())
{
this->setValue(arg->getReal());
if (arg->isSetUnits())
return;
}
if (source->isName())
{
dest->setType(source->getType());
dest->setName(source->getName());
}
else if (source->isReal())
{
dest->setValue(source->getReal());
if (source->isSetUnits())
{
this->setUnits(arg->getUnits());
dest->setUnits(source->getUnits());
}
}
else if (arg->isInteger())
{
this->setValue(arg->getInteger());
if (arg->isSetUnits())
}
else if (source->isInteger())
{
dest->setValue(source->getInteger());
if (source->isSetUnits())
{
this->setUnits(arg->getUnits());
dest->setUnits(source->getUnits());
}
}
else if (arg->isConstant())
{
this->setType(arg->getType());
}
else
{
this->setType(arg->getType());
this->setName(arg->getName());
for (unsigned int c = 0; c < arg->getNumChildren(); c++)
}
else if (source->isConstant())
{
dest->setType(source->getType());
}
else
{
dest->setType(source->getType());
dest->setName(source->getName());
for (unsigned int c = 0; c < source->getNumChildren(); c++)
{
this->addChild(arg->getChild(c)->deepCopy());
dest->addChild(source->getChild(c)->deepCopy());
}
}
}
}

LIBSBML_EXTERN
void
ASTNode::replaceArgument(const std::string& bvar, ASTNode * arg)
{
if (getNumChildren() == 0 && this->isName() && this->getName() == bvar)
{
copyNode(arg, this);
return;
}
for (unsigned int i = 0; i < getNumChildren(); i++)
{
if (getChild(i)->isName())
if (getChild(i)->isName() && getChild(i)->getName() == bvar)
{
if (getChild(i)->getName() == bvar)
{
if (arg->isName())
{
getChild(i)->setType(arg->getType());
getChild(i)->setName(arg->getName());
}
else if (arg->isReal())
{
getChild(i)->setValue(arg->getReal());
if (arg->isSetUnits())
{
getChild(i)->setUnits(arg->getUnits());
}
}
else if (arg->isInteger())
copyNode(arg, getChild(i));
}
else
{
getChild(i)->replaceArgument(bvar, arg);
}
}
}

LIBSBML_EXTERN
void
ASTNode::replaceArguments(const std::vector<std::string>& bvars, std::vector<ASTNode *>& args)
{
std::size_t n = bvars.size();
if (getNumChildren() == 0)
{
for(std::size_t j=0; j<n; ++j)
{
getChild(i)->setValue(arg->getInteger());
if (arg->isSetUnits())
{
getChild(i)->setUnits(arg->getUnits());
}
if (this->isName() && this->getName() == bvars[j])
{
copyNode(args[j], this);
return;
}
}
else if (arg->isConstant())
}
for (unsigned int i = 0; i < getNumChildren(); i++)
{
bool child_replaced = false;
for(std::size_t j=0; j<n; ++j)
{
getChild(i)->setType(arg->getType());
if (getChild(i)->isName() && getChild(i)->getName() == bvars[j])
{
copyNode(args[j], getChild(i));
child_replaced = true;
break;
}
}
else
if (!child_replaced)
{
getChild(i)->setType(arg->getType());
getChild(i)->setName(arg->getName());
for (unsigned int c = 0; c < arg->getNumChildren(); c++)
{
getChild(i)->addChild(arg->getChild(c)->deepCopy());
}
getChild(i)->replaceArguments(bvars, args);
}
}
}
else
{
getChild(i)->replaceArgument(bvar, arg);
}
}
}

LIBSBML_EXTERN
Expand Down
18 changes: 16 additions & 2 deletions src/sbml/math/ASTNode.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1796,7 +1796,7 @@ setValue(value, 0);
* For example, if the formula in this ASTNode is <code>x + y</code>,
* and the function is called with @c bvar = @c "x" and @c arg = an ASTNode
* representing the real value @c 3. This method would substitute @c 3 for
* @c x within this ASTNode object, resulting in the forula <code>3 + y</code>.
* @c x within this ASTNode object, resulting in the formula <code>3 + y</code>.
*
* @param bvar a string representing the variable name to be substituted.
* @param arg an ASTNode representing the name/value/formula to use as
Expand All @@ -1805,8 +1805,22 @@ setValue(value, 0);
LIBSBML_EXTERN
void replaceArgument(const std::string& bvar, ASTNode * arg);

/**
* Replaces occurrences of each given name with the corresponding ASTNode.
*
* For example, if the formula in this ASTNode is <code>x - y</code>,
* and the function is called with bvars = {"x", "y"} and args = ASTNodes
* representing objects with names {"y", "x"}, the result would be <code>y - x</code>.
*
* @param bvars a vector of strings representing the variable names to be substituted.
* @param args a vector of ASTNodes representing the name/value/formula to use as
* a replacement for each variable name
*/
LIBSBML_EXTERN
void replaceArguments(const std::vector<std::string>& bvars, std::vector<ASTNode *>& args);

/** @cond doxygenLibsbmlInternal */

/** @cond doxygenLibsbmlInternal */

/**
* Sets the parent SBML object of this node. Is not recursive, and will not set the parent SBML object of any children of this node.
Expand Down
6 changes: 3 additions & 3 deletions src/sbml/test/TestSBMLTransforms.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,9 @@ START_TEST (test_SBMLTransforms_replaceFD)
safe_free(math);

/* https://github.com/sbmlteam/libsbml/issues/299 */
/* fd: f_relabelled(p, S1) = p * S1 */
/* fd: f_relabelled(p, S1) = p - S1 */
/* ast: f_relabelled(S1, p) * compartmentOne / t */
/* ast after replaceFD: p * S1 * compartmentOne / t */
/* ast after replaceFD: (S1 - p) * compartmentOne / t */

// use initial assignment instead of reaction, because the reaction
// got flagged as having invalid units
Expand All @@ -114,7 +114,7 @@ START_TEST (test_SBMLTransforms_replaceFD)
SBMLTransforms::replaceFD(&ast, fd);

math = SBML_formulaToString(&ast);
fail_unless (!strcmp(math, "p * S1 * compartmentOne / t"), NULL);
fail_unless (!strcmp(math, "(S1 - p) * compartmentOne / t"), NULL);
safe_free(math);

/* one function definition - nested */
Expand Down
2 changes: 1 addition & 1 deletion src/sbml/test/test-data/multiple-functions.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
<ci> S1 </ci>
</bvar>
<apply>
<times/>
<minus/>
<ci> p </ci>
<ci> S1 </ci>
</apply>
Expand Down