From c4a131d719ede290e26eb94cd222c9d6d9000972 Mon Sep 17 00:00:00 2001 From: Sarah Keating Date: Tue, 28 Jan 2025 15:44:42 +0000 Subject: [PATCH] so in fixing one thing I've broken another --- src/sbml/conversion/ExpressionAnalyser.cpp | 80 ++++----- src/sbml/conversion/ExpressionAnalyser.h | 168 ++++++++++++++---- .../test/TestSBMLRateRuleConverter.cpp | 6 +- 3 files changed, 171 insertions(+), 83 deletions(-) diff --git a/src/sbml/conversion/ExpressionAnalyser.cpp b/src/sbml/conversion/ExpressionAnalyser.cpp index 0c4890b4e..14e12976f 100644 --- a/src/sbml/conversion/ExpressionAnalyser.cpp +++ b/src/sbml/conversion/ExpressionAnalyser.cpp @@ -71,7 +71,8 @@ ExpressionAnalyser::ExpressionAnalyser() mODEs (), mNewVarName("newVar"), mNewVarCount(1), - mHiddenSpecies (NULL) + mHiddenSpecies (NULL), + mHiddenNodes (NULL) { } @@ -82,7 +83,8 @@ ExpressionAnalyser::ExpressionAnalyser(Model * m, pairODEs odes) mODEs(odes), mNewVarName("newVar"), mNewVarCount(1), - mHiddenSpecies(NULL) + mHiddenSpecies(NULL), + mHiddenNodes(NULL) { SBMLTransforms::mapComponentValues(mModel); mModel->populateAllElementIdList(); @@ -93,7 +95,8 @@ ExpressionAnalyser::ExpressionAnalyser(const ExpressionAnalyser& orig) : mODEs(orig.mODEs), mNewVarName(orig.mNewVarName), mNewVarCount(orig.mNewVarCount), - mHiddenSpecies(orig.mHiddenSpecies) + mHiddenSpecies(orig.mHiddenSpecies), + mHiddenNodes(orig.mHiddenNodes) { SBMLTransforms::mapComponentValues(mModel); mModel->populateAllElementIdList(); @@ -112,6 +115,7 @@ ExpressionAnalyser::operator=(const ExpressionAnalyser& rhs) mNewVarName = rhs.mNewVarName; mNewVarCount = rhs.mNewVarCount; mHiddenSpecies = rhs.mHiddenSpecies; + mHiddenNodes = rhs.mHiddenNodes; } SBMLTransforms::mapComponentValues(mModel); mModel->populateAllElementIdList(); @@ -134,6 +138,7 @@ ExpressionAnalyser::~ExpressionAnalyser () // note the odes are owned by the converter SBMLTransforms::clearComponentValues(mModel); mHiddenSpecies = NULL; + mHiddenNodes = NULL; if (mExpressions.size() > 0) { mExpressions.clear(); @@ -144,8 +149,8 @@ void ExpressionAnalyser::identifyHiddenSpeciesWithinExpressions() { if (mExpressions.empty()) return; - if (mHiddenSpecies == NULL) - mHiddenSpecies = new List(); + if (mHiddenNodes == NULL) + mHiddenNodes = new List(); // need to actually address the expressions in the correct order // replace k-x-y first with newParam type=TYPE_K_MINUS_X_MINUS_Y @@ -238,32 +243,9 @@ void ExpressionAnalyser::identifyHiddenSpeciesWithinExpressions() addNewParameterPlusVOrW(exp, "v"); } } - //else if (exp->type == TYPE_K_PLUS_V_MINUS_X) - //{ - // if (mExpressions[j - 1]->type == TYPE_K_MINUS_X) - // { - // // here we are dealing with the fact that we have k+v-x and k-x - //// need to check whether they have the same values for k,x - // if (matchesK(mExpressions[j - 1], exp) && - // matchesXValue(mExpressions[j - 1], exp)) - // { - // // the values are the same so we can use the same new parameter - // addPreviousParameterPlusVOrW(exp, mExpressions[j - 1], "v"); - // } - // else - // { - // // if the type before was not k-x then we need to create a new parameter - // addNewParameterPlusVOrW(exp, "v"); - // } - //} - //else - //{ - // // if the type before was not k-x then we need to create a new parameter - // // because it doesn't matter what it was. - // addNewParameterPlusVOrW(exp, "v"); - //} - } + } } + substituteNodes(); } void ExpressionAnalyser::substituteParameters(SubstitutionValues_t* exp) @@ -337,7 +319,7 @@ void ExpressionAnalyser::addSingleNewParameter(SubstitutionValues_t* exp) ASTNode* z = new ASTNode(AST_NAME); z->setName(zName.c_str()); exp->z_expression = z->deepCopy(); - mHiddenSpecies->add(z); + mHiddenNodes->add(z); delete z; } @@ -354,7 +336,7 @@ void ExpressionAnalyser::addNewParameterPlusVOrW(SubstitutionValues_t* exp, std: replacement->addChild(z); replacement->addChild(var); exp->z_expression = replacement->deepCopy(); - mHiddenSpecies->add(z); + mHiddenNodes->add(z); delete replacement; } @@ -373,6 +355,7 @@ void ExpressionAnalyser::addPreviousParameterPlusVOrW(SubstitutionValues_t* exp, } + /* * Check whether two SubstitutionValues_t match the values that we expect if we need to add them * based on the type of the SubstitutionValues_t @@ -517,12 +500,6 @@ bool ExpressionAnalyser::matchesDydtExpression(SubstitutionValues_t* values1, Su values1->dydt_expression->exactlyEqual(*(values2->dydt_expression)) == true); } -bool ExpressionAnalyser::matchesCurrentNode(SubstitutionValues_t* values1, SubstitutionValues_t* values2) -{ - return (values1->current != NULL && values2->current != NULL && - values1->current->exactlyEqual(*(values2->current)) == true); -} - bool ExpressionAnalyser::matchesType(SubstitutionValues_t* values1, SubstitutionValues_t* values2) { return values1->type == values2->type; @@ -667,6 +644,17 @@ ExpressionAnalyser::getODEFor(std::string name) zero->setValue(0.0); return zero->deepCopy(); } + +ASTNode* +ExpressionAnalyser::getODE(unsigned int odeIndex) +{ + if (odeIndex < mODEs.size()) + { + return mODEs.at(odeIndex).second; + } + return nullptr; +} + void ExpressionAnalyser::analyse() { @@ -731,9 +719,9 @@ ExpressionAnalyser::replaceExpressionInNodeWithNode(ASTNode* node, ASTNode* repl { return; } - //cout << "node: " << SBML_formulaToL3String(node) << endl; - //cout << "with: " << SBML_formulaToL3String(replaced) << endl; - //cout << "by: " << SBML_formulaToL3String(replacement) << endl; + cout << "node: " << SBML_formulaToL3String(node) << endl; + cout << "with: " << SBML_formulaToL3String(replaced) << endl; + cout << "by: " << SBML_formulaToL3String(replacement) << endl; // we might be replcing the whole node if (node == replaced) { @@ -773,6 +761,16 @@ ExpressionAnalyser::getUniqueNewParameterName() return mNewVarName + std::to_string(mNewVarCount); } +void ExpressionAnalyser::substituteNodes() +{ + for (unsigned int i = 0; i < mExpressions.size(); i++) + { + SubstitutionValues_t* exp = mExpressions.at(i); + replaceExpressionInNodeWithNode(exp->current, exp->z_expression, getODE(exp->odeIndex)); //getODEFor(exp->current); + cout << "node: " << SBML_formulaToL3String(getODE(exp->odeIndex)) << endl; + } +} + void ExpressionAnalyser::addParametersAndRateRules(SubstitutionValues_t* exp) diff --git a/src/sbml/conversion/ExpressionAnalyser.h b/src/sbml/conversion/ExpressionAnalyser.h index 41ee369e4..06dad87ef 100644 --- a/src/sbml/conversion/ExpressionAnalyser.h +++ b/src/sbml/conversion/ExpressionAnalyser.h @@ -100,7 +100,7 @@ class LIBSBML_EXTERN ExpressionAnalyser { public: - + /** * Creates a new ExpressionAnalyser object. */ @@ -206,27 +206,68 @@ class LIBSBML_EXTERN ExpressionAnalyser private: /** @cond doxygenLibsbmlInternal */ + + /** + * Function to match two substitution values matching the k parameter + */ bool matchesK(SubstitutionValues_t* values1, SubstitutionValues_t* values2); + /** + * Function to match two substitution values matching the k parameter string value + */ bool matchesKValue(SubstitutionValues_t* values1, SubstitutionValues_t* values2); + + /** + * Function to match two substitution values matching the k parameter numerical value + */ bool matchesKRealValue(SubstitutionValues_t* values1, SubstitutionValues_t* values2); + + /** + * Function to match two substitution values matching the x parameter + */ bool matchesXValue(SubstitutionValues_t* values1, SubstitutionValues_t* values2); + + /** + * Function to match two substitution values matching the y parameter + */ bool matchesYValue(SubstitutionValues_t* values1, SubstitutionValues_t* values2); + + /** + * Function to match two substitution values matching the v expression + */ bool matchesVExpression(SubstitutionValues_t* values1, SubstitutionValues_t* values2); + + /** + * Function to match two substitution values matching the w expression + */ bool matchesWExpression(SubstitutionValues_t* values1, SubstitutionValues_t* values2); + + /** + * Function to match two substitution values matching the dx/dt expression + */ bool matchesDxdtExpression(SubstitutionValues_t* values1, SubstitutionValues_t* values2); - bool matchesDydtExpression(SubstitutionValues_t* values1, SubstitutionValues_t* values2); - bool matchesCurrentNode(SubstitutionValues_t* values1, SubstitutionValues_t* values2); - bool matchesType(SubstitutionValues_t* values1, SubstitutionValues_t* values2); + /** + * Function to match two substitution values matching the dy/dt expression + */ + bool matchesDydtExpression(SubstitutionValues_t* values1, SubstitutionValues_t* values2); - void substituteParameters(SubstitutionValues_t* values); + /** + * Function to match two substitution values matching the type + */ + bool matchesType(SubstitutionValues_t* values1, SubstitutionValues_t* values2); + /** + * Function to get the substitution values by type + */ SubstitutionValues_t* getSubstitutionValuesByType(ExpressionType_t type); + /** + * Function to get the matching parent expression for the given expression + * suppose current expression is k-x+w-y it's parent expression will be k-x-y + */ int getMatchingParentExpression(SubstitutionValues_t* value, unsigned int index); - // functions that represents steps of algo 3.1 void addSingleNewParameter(SubstitutionValues_t* exp); @@ -240,41 +281,28 @@ class LIBSBML_EXTERN ExpressionAnalyser */ ASTNode* getODEFor(std::string name); - /* - * THIS NEEDS PROPERLY SORTING IS DO THE FUNCTIONS THAT DEAL WITH MATCHING WHETHER A PARAMETER IS ALREADY IN THE MODEL - * ALSO NEED TO TAKE THE MATCHING OF THE ODES OUT OF THE COMPARISON OF EXPRESSIONS - * AND CHECK THAT THE MATCHES VARIABLES INCLUDES ALL THE VARIABLES - */ - void addParametersAndRateRules(SubstitutionValues_t* exp = NULL); + ASTNode* getODE(unsigned int odeIndex); - void replaceExpressionInNodeWithNode(ASTNode* node, ASTNode* replaced, ASTNode* replacement); - - void replaceExpressionInNodeWithVar(ASTNode* node, ASTNode* replaced, std::string var); - - std::string getUniqueNewParameterName(); - - - - void replaceExpressionWithNewParameter(ASTNode* ode, SubstitutionValues_t* exp); /* - * Loops through expressions already recorded and checks for exact matches + * Check whether the expression has a parent expression which may already have been analysed + * in which case we do not need to re analyse the child expression + * e.g. if we have k-x-y do not need to analyse k-x + */ + bool parentExpressionExists(SubstitutionValues_t* current, SubstitutionValues_t* mightAdd); + + /* + * Check whether the expression should be added to the list of expressions + * This involves checking whether it has a parent expression which may already have been analysed + * and checking whether it already exists */ bool shouldAddExpression(SubstitutionValues_t* value, ASTNodePair currentNode); + /* + * Check whether the expression already exists in the list of expressions + */ bool expressionExists(SubstitutionValues_t* current, SubstitutionValues_t* mightAdd); - bool parentExpressionExists(SubstitutionValues_t* current, SubstitutionValues_t* mightAdd); - - /** - * Searches for a node's parent and its index as the parent's child in a one-directional tree (nodes know their children, but not their parent). - * E.g. if the node is the first child of a node, this function will return a pair (parent, 0). - * - * @param child node whose parent should be found - * @param root root node of the tree to search - * @return pair of parent and index - or (nullptr, NAN) if not found. - */ - std::pair getParentNode(const ASTNode* child, const ASTNode* root); /** * Checks whether a node is a variable species or a variable parameter in a model. @@ -291,36 +319,95 @@ class LIBSBML_EXTERN ExpressionAnalyser * @return true if the node is a constant number/parameter */ bool isNumericalConstantOrConstantParameter(ASTNode* node, bool& isNumber); - + /* - * Checks whether a parameter with the given name is already in the model. - * - * @param name the name of the parameter to check - * @return true if the parameter is already in the model, false otherwise.flac + * Function to get a unique name for a new parameter */ - bool isParameterAlreadyCreated(std::string& name); + std::string getUniqueNewParameterName(); + void substituteNodes(); + /* + * Function to check whether the expressions are of the form k-x-y + */ bool isTypeKminusXminusY(unsigned int numChildren, ASTNode* rightChild, ASTNode* leftChild, ASTNodeType_t type, SubstitutionValues_t* value); + + /* + * Function to check whether the expressions are of the form k-x + */ bool isTypeKminusX(unsigned int numChildren, ASTNode* rightChild, ASTNode* leftChild, ASTNodeType_t type, SubstitutionValues_t* value); + + /* + * Function to check whether the expressions are of the form k+v-x + */ bool isTypeKplusVminusX(unsigned int numChildren, ASTNode* rightChild, ASTNode* leftChild, ASTNodeType_t type, SubstitutionValues_t* value); + + /* + * Function to check whether the expressions are of the form k+v + */ bool isTypeKplusV(unsigned int numChildren, ASTNode* rightChild, ASTNode* leftChild, ASTNodeType_t type, SubstitutionValues_t* value); + /* + * Function to check whether the expressions are of the form k+v-x-y + */ bool isTypeKplusVminusXminusY(unsigned int numChildren, ASTNode* rightChild, ASTNode* leftChild, ASTNodeType_t type, SubstitutionValues_t* value); + + /* + * Function to check whether the expressions are of the form k-x+w-y + */ bool isTypeKminusXplusWminusY(unsigned int numChildren, ASTNode* rightChild, ASTNode* leftChild, ASTNodeType_t type, SubstitutionValues_t* value); + + /* + * Function to check whether the expressions are of the form k+w-x + */ bool isTypeWplusKminusX(unsigned int numChildren, ASTNode* rightChild, ASTNode* leftChild, ASTNodeType_t type, SubstitutionValues_t* value); + // to do see if this is necessary ==================================================== + /** + * Searches for a node's parent and its index as the parent's child in a one-directional tree (nodes know their children, but not their parent). + * E.g. if the node is the first child of a node, this function will return a pair (parent, 0). + * + * @param child node whose parent should be found + * @param root root node of the tree to search + * @return pair of parent and index - or (nullptr, NAN) if not found. + */ + std::pair getParentNode(const ASTNode* child, const ASTNode* root); + + /* +* Checks whether a parameter with the given name is already in the model. +* +* @param name the name of the parameter to check +* @return true if the parameter is already in the model, false otherwise.flac +*/ + bool isParameterAlreadyCreated(std::string& name); + + + void substituteParameters(SubstitutionValues_t* values); + /* +* THIS NEEDS PROPERLY SORTING IS DO THE FUNCTIONS THAT DEAL WITH MATCHING WHETHER A PARAMETER IS ALREADY IN THE MODEL +* ALSO NEED TO TAKE THE MATCHING OF THE ODES OUT OF THE COMPARISON OF EXPRESSIONS +* AND CHECK THAT THE MATCHES VARIABLES INCLUDES ALL THE VARIABLES +*/ + void addParametersAndRateRules(SubstitutionValues_t* exp = NULL); + + void replaceExpressionInNodeWithNode(ASTNode* node, ASTNode* replaced, ASTNode* replacement); + + void replaceExpressionInNodeWithVar(ASTNode* node, ASTNode* replaced, std::string var); + + void replaceExpressionWithNewParameter(ASTNode* ode, SubstitutionValues_t* exp); + + // member variables populated during analysis pairODEs mODEs; @@ -331,6 +418,9 @@ class LIBSBML_EXTERN ExpressionAnalyser // list of hidden species that are identified during the analysis List* mHiddenSpecies; + // list of hidden nodes that are identified during the analysis + List* mHiddenNodes; + // variables to ensure unique new parameter name std::string mNewVarName; diff --git a/src/sbml/conversion/test/TestSBMLRateRuleConverter.cpp b/src/sbml/conversion/test/TestSBMLRateRuleConverter.cpp index 34eecfe69..cb6a64ef2 100644 --- a/src/sbml/conversion/test/TestSBMLRateRuleConverter.cpp +++ b/src/sbml/conversion/test/TestSBMLRateRuleConverter.cpp @@ -413,7 +413,7 @@ START_TEST(test_conversion_raterule_converter_hidden_variable) fail_unless(doc->getModel()->getNumRules() == 4); fail_unless(doc->getModel()->getNumReactions() == 0); - converter->setDocument(doc); + converter->setDocument(doc); fail_unless(converter->convert() == LIBSBML_OPERATION_SUCCESS); fail_unless(doc->getModel()->getNumCompartments() == 1); @@ -1041,12 +1041,12 @@ END_TEST Suite * create_suite_TestSBMLRateRuleConverter (void) { - bool testing = false; + bool testing = true; Suite *suite = suite_create("SBMLRateRuleConverter"); TCase *tcase = tcase_create("SBMLRateRuleConverter"); if (testing) { - tcase_add_test(tcase, test_conversion_raterule_converter_invalid); + tcase_add_test(tcase, test_conversion_raterule_converter_hidden_variable); } else {