From 8f723a402e21293757bcf43b44089c0a36a787af Mon Sep 17 00:00:00 2001 From: Hugh Sorby Date: Tue, 13 Oct 2026 09:16:19 +1300 Subject: [PATCH 1/9] Add tests illustrating issue with parser parsing connections. --- tests/connection/connection.cpp | 52 +++++++++++++++++++ tests/resources/cellml1X/connections.cellml | 56 +++++++++++++++++++++ 2 files changed, 108 insertions(+) create mode 100644 tests/resources/cellml1X/connections.cellml diff --git a/tests/connection/connection.cpp b/tests/connection/connection.cpp index 4789fe4b7f..cd7a9a695e 100644 --- a/tests/connection/connection.cpp +++ b/tests/connection/connection.cpp @@ -1535,3 +1535,55 @@ TEST(Connection, addEquivalenceConnectionIdClearedAfterDisconnect) libcellml::Variable::addEquivalence(v1, v2); EXPECT_EQ("", libcellml::Variable::equivalenceConnectionId(v1, v2)); } + +TEST(Connection, twoWayConnectionFromCellML1) +{ + auto p = libcellml::Parser::create(false); + auto m = p->parseModel(fileContents("cellml1X/connections.cellml")); + + EXPECT_EQ(size_t(0), p->errorCount()); +} + +TEST(Connection, sameNamedVariablesInDifferentComponents) +{ + auto m = libcellml::Model::create("m"); + auto c1 = libcellml::Component::create("c1"); + auto c2 = libcellml::Component::create("c2"); + auto v1c1 = libcellml::Variable::create("v1"); + auto v2c1 = libcellml::Variable::create("v2"); + auto v1c2 = libcellml::Variable::create("v1"); + auto v2c2 = libcellml::Variable::create("v2"); + + m->addComponent(c1); + m->addComponent(c2); + + c1->addVariable(v1c1); + c1->addVariable(v2c1); + c2->addVariable(v1c2); + c2->addVariable(v2c2); + + v1c1->setUnits("dimensionless"); + v2c1->setUnits("dimensionless"); + v1c2->setUnits("dimensionless"); + v2c2->setUnits("dimensionless"); + + libcellml::Variable::addEquivalence(v1c1, v2c2); + libcellml::Variable::addEquivalence(v2c1, v1c2); + + auto v = libcellml::Validator::create(); + v->validateModel(m); + + EXPECT_EQ(size_t(0), v->issueCount()); + + auto printer = libcellml::Printer::create(); + + auto modelString = printer->printModel(m); + + Debug() << modelString; + + auto p = libcellml::Parser::create(); + p->parseModel(modelString); + + printIssues(p); + EXPECT_EQ(size_t(0), p->issueCount()); +} diff --git a/tests/resources/cellml1X/connections.cellml b/tests/resources/cellml1X/connections.cellml new file mode 100644 index 0000000000..4c428e33be --- /dev/null +++ b/tests/resources/cellml1X/connections.cellml @@ -0,0 +1,56 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file From 66d6bd3b59347d68086d4a7c3404fb592dc032bf Mon Sep 17 00:00:00 2001 From: Hugh Sorby Date: Tue, 13 Oct 2026 11:47:04 +1300 Subject: [PATCH 2/9] Fix up sameNamedVariablesInDifferentComponents so it creates a validator valid model. --- tests/connection/connection.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/connection/connection.cpp b/tests/connection/connection.cpp index cd7a9a695e..76fda64dd6 100644 --- a/tests/connection/connection.cpp +++ b/tests/connection/connection.cpp @@ -1435,7 +1435,7 @@ TEST(Connection, repeatedMapVariables) const std::vector expectedIssues = { "Connection in model 'connection' between 'variable1' and 'variable2' is not unique.", - "Connection in model 'connection' between 'variable3' and 'variable6' is not unique.", + "Connection in model 'connection' between 'variable6' and 'variable3' is not unique.", }; EXPECT_EQ_ISSUES(expectedIssues, p); @@ -1563,9 +1563,13 @@ TEST(Connection, sameNamedVariablesInDifferentComponents) c2->addVariable(v2c2); v1c1->setUnits("dimensionless"); + v1c1->setInterfaceType("public"); v2c1->setUnits("dimensionless"); + v2c1->setInterfaceType("public"); v1c2->setUnits("dimensionless"); + v1c2->setInterfaceType("public"); v2c2->setUnits("dimensionless"); + v2c2->setInterfaceType("public"); libcellml::Variable::addEquivalence(v1c1, v2c2); libcellml::Variable::addEquivalence(v2c1, v1c2); @@ -1579,11 +1583,8 @@ TEST(Connection, sameNamedVariablesInDifferentComponents) auto modelString = printer->printModel(m); - Debug() << modelString; - auto p = libcellml::Parser::create(); p->parseModel(modelString); - printIssues(p); EXPECT_EQ(size_t(0), p->issueCount()); } From 9301fb62863b23864d8a4d3db6f14ef45e834a58 Mon Sep 17 00:00:00 2001 From: Hugh Sorby Date: Tue, 13 Oct 2026 11:47:33 +1300 Subject: [PATCH 3/9] Don't canonaclise map variables in connection becuase the order is important. --- src/parser.cpp | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/parser.cpp b/src/parser.cpp index 7728f4ccd6..01e009edad 100644 --- a/src/parser.cpp +++ b/src/parser.cpp @@ -1240,11 +1240,6 @@ void Parser::ParserImpl::loadConnection(const ModelPtr &model, const XmlNodePtr mapVariablesFound = true; if (!variable1Missing && !variable2Missing) { - if (variable1Name > variable2Name) { - std::string tmp = variable1Name; - variable1Name = variable2Name; - variable2Name = tmp; - } auto variableNamePair = std::make_pair(variable1Name, variable2Name); From 7b71eb37e5533e2ece83715e1bcb9e1aa82300eb Mon Sep 17 00:00:00 2001 From: Hugh Sorby Date: Tue, 13 Oct 2026 11:48:14 +1300 Subject: [PATCH 4/9] Fix formatting in src/parser.cpp. --- src/parser.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/parser.cpp b/src/parser.cpp index 01e009edad..3060c83f98 100644 --- a/src/parser.cpp +++ b/src/parser.cpp @@ -1240,7 +1240,6 @@ void Parser::ParserImpl::loadConnection(const ModelPtr &model, const XmlNodePtr mapVariablesFound = true; if (!variable1Missing && !variable2Missing) { - auto variableNamePair = std::make_pair(variable1Name, variable2Name); NamePairList::const_iterator it = std::find_if(usedMapVariables.begin(), usedMapVariables.end(), From 3da31d9f5c72df749dfa67213f88219a65d90983 Mon Sep 17 00:00:00 2001 From: Hugh Sorby Date: Tue, 13 Oct 2026 15:12:14 +1300 Subject: [PATCH 5/9] Add a test to examine the bahaviour of map components parsing with CellML 1.X models. --- tests/connection/connection.cpp | 8 +++++ .../resources/cellml1X/map_components.cellml | 30 +++++++++++++++++++ 2 files changed, 38 insertions(+) create mode 100644 tests/resources/cellml1X/map_components.cellml diff --git a/tests/connection/connection.cpp b/tests/connection/connection.cpp index 76fda64dd6..abefc260c9 100644 --- a/tests/connection/connection.cpp +++ b/tests/connection/connection.cpp @@ -1588,3 +1588,11 @@ TEST(Connection, sameNamedVariablesInDifferentComponents) EXPECT_EQ(size_t(0), p->issueCount()); } + +TEST(Connection, mapComponentsAreOrderDependentInCellML1X) +{ + auto p = libcellml::Parser::create(false); + auto m = p->parseModel(fileContents("cellml1X/map_components.cellml")); + + EXPECT_EQ(size_t(0), p->errorCount()); +} diff --git a/tests/resources/cellml1X/map_components.cellml b/tests/resources/cellml1X/map_components.cellml new file mode 100644 index 0000000000..7de46b31c6 --- /dev/null +++ b/tests/resources/cellml1X/map_components.cellml @@ -0,0 +1,30 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + From 9013fb941759d14968eed818526458f2ccb569b0 Mon Sep 17 00:00:00 2001 From: Hugh Sorby Date: Tue, 13 Oct 2026 15:26:52 +1300 Subject: [PATCH 6/9] Ignore order when parsing map components in CellML1.X models. --- src/parser.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/parser.cpp b/src/parser.cpp index 3060c83f98..04f1408417 100644 --- a/src/parser.cpp +++ b/src/parser.cpp @@ -1109,7 +1109,7 @@ void Parser::ParserImpl::loadConnection(const ModelPtr &model, const XmlNodePtr issue->mPimpl->setReferenceRule(Issue::ReferenceRule::CONNECTION_EXCLUDE_SELF); addIssue(issue); } else { - if (component1Name > component2Name) { + if (!mParsing1XVersion && (component1Name > component2Name)) { std::string tmp = component1Name; component1Name = component2Name; component2Name = tmp; From cb0d3cd169f5bf37641a7ca7bb19e08f84c7e482 Mon Sep 17 00:00:00 2001 From: Hugh Sorby Date: Wed, 14 Oct 2026 15:27:20 +1300 Subject: [PATCH 7/9] Add a test with a CellML 1.1 model that defines multiple encapsulations. --- tests/parser/cellml_1_1.cpp | 15 ++++++++++ .../cellml1X/multiple_encapsulations.cellml | 30 +++++++++++++++++++ 2 files changed, 45 insertions(+) create mode 100644 tests/resources/cellml1X/multiple_encapsulations.cellml diff --git a/tests/parser/cellml_1_1.cpp b/tests/parser/cellml_1_1.cpp index cb5a486a1f..92373fe0b0 100644 --- a/tests/parser/cellml_1_1.cpp +++ b/tests/parser/cellml_1_1.cpp @@ -141,3 +141,18 @@ TEST(ParserTransform, cellmlNsCn) EXPECT_EQ(size_t(0), validator->issueCount()); } + +TEST(ParserTransform, parseCellMl1XWithMultipleEncapsulationRelationshipGroups) +{ + std::vector expectedIssues = { + "Given model is a CellML 1.1 model, the parser will try to represent this model in CellML 2.0.", + }; + + auto parser = libcellml::Parser::create(false); + auto model = parser->parseModel(fileContents("cellml1X/multiple_encapsulations.cellml")); + + printIssues(parser); + + EXPECT_EQ_ISSUES(expectedIssues, parser); + EXPECT_EQ("Example_MassAction", model->name()); +} diff --git a/tests/resources/cellml1X/multiple_encapsulations.cellml b/tests/resources/cellml1X/multiple_encapsulations.cellml new file mode 100644 index 0000000000..5de46c60b7 --- /dev/null +++ b/tests/resources/cellml1X/multiple_encapsulations.cellml @@ -0,0 +1,30 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + From 3542bd3807b51cabb81aa9d3a1b480d8a54a1e32 Mon Sep 17 00:00:00 2001 From: Hugh Sorby Date: Wed, 14 Oct 2026 15:31:20 +1300 Subject: [PATCH 8/9] Add new line at end of connections.cellml. --- tests/resources/cellml1X/connections.cellml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/resources/cellml1X/connections.cellml b/tests/resources/cellml1X/connections.cellml index 4c428e33be..1890e9a337 100644 --- a/tests/resources/cellml1X/connections.cellml +++ b/tests/resources/cellml1X/connections.cellml @@ -53,4 +53,4 @@ - \ No newline at end of file + From 555f14074fb85dab956c0cea0132d3cf9881201f Mon Sep 17 00:00:00 2001 From: Hugh Sorby Date: Fri, 16 Oct 2026 10:09:53 +1300 Subject: [PATCH 9/9] Improve the way multiple encapsulations are dealt with by the parser. --- src/parser.cpp | 19 ++++++++++--------- tests/parser/parser.cpp | 1 + 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/src/parser.cpp b/src/parser.cpp index 04f1408417..14d3be844b 100644 --- a/src/parser.cpp +++ b/src/parser.cpp @@ -590,15 +590,16 @@ void Parser::ParserImpl::loadModel(const ModelPtr &model, const std::string &inp childNode = childNode->next(); } - if (!encapsulationNodes.empty()) { - loadEncapsulation(model, encapsulationNodes.at(0)); - if (encapsulationNodes.size() > 1) { - auto issue = Issue::IssueImpl::create(); - issue->mPimpl->setDescription("Model '" + model->name() + "' has more than one encapsulation element."); - issue->mPimpl->mItem->mPimpl->setEncapsulation(model); - issue->mPimpl->setReferenceRule(Issue::ReferenceRule::MODEL_MORE_THAN_ONE_ENCAPSULATION); - addIssue(issue); - } + for (const auto &encapsulationNode : encapsulationNodes) { + loadEncapsulation(model, encapsulationNode); + } + + if (mParsing20Version && encapsulationNodes.size() > 1) { + auto issue = Issue::IssueImpl::create(); + issue->mPimpl->setDescription("Model '" + model->name() + "' has more than one encapsulation element."); + issue->mPimpl->mItem->mPimpl->setEncapsulation(model); + issue->mPimpl->setReferenceRule(Issue::ReferenceRule::MODEL_MORE_THAN_ONE_ENCAPSULATION); + addIssue(issue); } NamePairList usedConnections; diff --git a/tests/parser/parser.cpp b/tests/parser/parser.cpp index 531cb3c67c..87bc95b599 100644 --- a/tests/parser/parser.cpp +++ b/tests/parser/parser.cpp @@ -956,6 +956,7 @@ TEST(Parser, invalidEncapsulations) "Encapsulation in model 'ringo' does not have a valid component attribute in a component_ref element.", "Encapsulation in model 'ringo' does not have a valid component attribute in a component_ref element.", "Encapsulation in model 'ringo' specifies an invalid parent component_ref that also does not have any children.", + "Encapsulation in model 'ringo' specifies 'bob' as a parent component_ref but it does not have any children.", "Model 'ringo' has more than one encapsulation element.", };