Skip to content

Commit

Permalink
Minimal example showing segfault on model flattening over multiple di…
Browse files Browse the repository at this point in the history
…rectories (with some debugging).
  • Loading branch information
hsorby committed Nov 20, 2024
1 parent d84057b commit c9f69b1
Show file tree
Hide file tree
Showing 12 changed files with 203 additions and 107 deletions.
2 changes: 1 addition & 1 deletion src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ set_target_properties(cellml_debug_utilities PROPERTIES
VISIBILITY_INLINES_HIDDEN 1
)

target_link_libraries(cellml PRIVATE debug $<BUILD_INTERFACE:cellml_debug_utilities>)
target_link_libraries(cellml PUBLIC debug $<BUILD_INTERFACE:cellml_debug_utilities>)

group_source_to_dir_structure(
${DEBUG_SOURCE_FILES}
Expand Down
98 changes: 98 additions & 0 deletions src/debug.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -768,4 +768,102 @@ void listModelsUnits(const ModelPtr &model)
}
}

static const std::string FIXED_INDENT = " ";

void printComponent(const libcellml::ComponentPtr &component, size_t c, const std::string &indent, bool includeMaths)
{
if (c == -1) {
std::cout << "COMPONENT: '" << component->name() << "'";
} else {
std::cout << indent << "[" << c + 1 << "]: " << component->name();
}

if (component->id() != "") {
std::cout << ", id: " << component->id();
}

std::cout << std::endl;
std::cout << indent << FIXED_INDENT << "VARIABLES: " << component->variableCount() << " variables" << std::endl;

// Printing the variables within the component.
for (size_t v = 0; v < component->variableCount(); ++v) {
std::cout << indent << FIXED_INDENT << FIXED_INDENT;
std::cout << "[" << v + 1 << "]: " << component->variable(v)->name();
if (component->variable(v)->units() != nullptr) {
std::cout << " [" << component->variable(v)->units()->name() << "]";
}
if (component->variable(v)->initialValue() != "") {
std::cout << ", initial = " << component->variable(v)->initialValue();
}
std::cout << std::endl;
if (component->variable(v)->equivalentVariableCount() > 0) {
std::cout << indent << FIXED_INDENT << FIXED_INDENT << FIXED_INDENT;
std::string con = " └──> ";
for (size_t e = 0; e < component->variable(v)->equivalentVariableCount(); ++e) {
auto ev = component->variable(v)->equivalentVariable(e);
if (ev == nullptr) {
std::cout << "WHOOPS! Null equivalent variable!";
continue;
}
libcellml::ComponentPtr ev_parent = std::dynamic_pointer_cast<libcellml::Component>(ev->parent());
if (ev_parent == nullptr) {
std::cout << "WHOOPS! Null parent component for equivalent variable!";
continue;
}
std::cout << con << ev_parent->name() << ":" << ev->name();
if (ev->units() != nullptr) {
std::cout << " [" << ev->units()->name() << "]";
}
con = ", ";
}
std::cout << std::endl;
}
}

// Print the maths within the component.
if (includeMaths) {
if (component->math() != "") {
std::cout << indent << " Maths in the component is:" << std::endl;
std::cout << component->math() << std::endl;
}
}

// Print the encapsulated components
if (component->componentCount() > 0) {
std::cout << indent << FIXED_INDENT << "CHILD COMPONENTS: " << component->componentCount()
<< " child components" << std::endl;
std::string newIndent = indent + FIXED_INDENT + FIXED_INDENT;

for (size_t c2 = 0; c2 < component->componentCount(); ++c2) {
auto child = component->component(c2);
printComponent(child, c2, newIndent, includeMaths);
}
}
}

void printComponent(const ComponentPtr &component, bool includeMaths)
{
printComponent(component, -1, {}, includeMaths);
}

void printModel(const ModelPtr &model, bool includeMaths)
{
std::cout << "MODEL: '" << model->name() << "'";
if (model->id() != "") {
std::cout << ", id: '" << model->id() << "'";
}
std::cout << std::endl;

std::cout << FIXED_INDENT << "UNITS: " << model->unitsCount() << " custom units" << std::endl;
for (size_t u = 0; u < model->unitsCount(); ++u) {
std::cout << FIXED_INDENT << FIXED_INDENT << "[" << u + 1 << "]: " << model->units(u)->name() << std::endl;
}

std::cout << FIXED_INDENT << "COMPONENTS: " << model->componentCount() << " components" << std::endl;
for (size_t c = 0; c < model->componentCount(); ++c) {
auto component = model->component(c);
printComponent(component, c, FIXED_INDENT + FIXED_INDENT, includeMaths);
}
}

} // namespace libcellml
10 changes: 6 additions & 4 deletions src/debug.h
Original file line number Diff line number Diff line change
Expand Up @@ -93,24 +93,26 @@ struct Debug
bool mNewLine;
};

void listModelsUnits(const ModelPtr &model);
void printAnalyserModelEquations(const AnalyserModelPtr &model);
void printAnalyserModelVariables(const AnalyserModelPtr &model);
void printAstAsTree(const AnalyserEquationAstPtr &ast);
void printAstAsCode(const AnalyserEquationAstPtr &ast);
void printComponent(const ComponentPtr &component, bool includeMaths = true);
void printComponentMap(const ComponentMap &map);
void printConnectionMap(const ConnectionMap &map);
void printEquivalenceMap(const EquivalenceMap &map);
void printEquivalenceMapWithModelInfo(const EquivalenceMap &map, const ModelPtr &model);
void printEquivalences(const VariablePtr &variable);
void printHistory(const History &history);
void printHistoryEpoch(const HistoryEpochPtr &historyEpoch);
void printImportLibrary(const ImportLibrary &importlibrary);
void printModel(const ModelPtr &model, bool includeMaths = true);
void printNamedPath(const ParentedEntityPtr &parented);
void printStack(const IndexStack &stack);
void printStackWithModelInfo(const IndexStack &stack, const ModelPtr &model);
void printStringStringMap(const StringStringMap &map);
void printVariableMap(const VariableMap &map);
void printUnits(const UnitsPtr &units);
void listModelsUnits(const ModelPtr &model);
void printNamedPath(const ParentedEntityPtr &parented);
void printEquivalences(const VariablePtr &variable);
void printVariableMap(const VariableMap &map);

} // namespace libcellml
15 changes: 14 additions & 1 deletion src/importer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ limitations under the License.

#include <algorithm>
#include <cmath>
#include <filesystem>
#include <fstream>
#include <libxml/uri.h>
#include <sstream>
Expand All @@ -37,6 +38,8 @@ limitations under the License.
#include "logger_p.h"
#include "utilities.h"

#include "debug.h"

namespace libcellml {

/**
Expand Down Expand Up @@ -329,14 +332,18 @@ std::string pathFromUrl(const std::string &url)
*/
std::string resolvePath(const std::string &filename, const std::string &base)
{
return pathFromUrl(base) + filename;
return std::filesystem::canonical(pathFromUrl(base) + filename).generic_string();
}

bool Importer::ImporterImpl::fetchModel(const ImportSourcePtr &importSource, const std::string &baseFile)
{
std::string url = normaliseDirectorySeparator(importSource->url());
Debug() << "Fetch model: " << importSource->url();
Debug() << " norm: " << url;
if (mLibrary.count(url) == 0) {
url = resolvePath(url, baseFile);
Debug() << "Resolved path: " << url;
Debug() << "now: " << std::filesystem::canonical(url);
}

ModelPtr model;
Expand Down Expand Up @@ -481,6 +488,7 @@ bool Importer::ImporterImpl::fetchComponent(const ComponentPtr &importComponent,
}

std::string resolvingUrl = ImporterImpl::resolvingUrl(importComponent->importSource());
Debug() << "Component -> resolvingUrl: " << resolvingUrl;

if (encounteredRelatedError) {
auto issue = Issue::IssueImpl::create();
Expand Down Expand Up @@ -572,6 +580,7 @@ bool Importer::ImporterImpl::fetchUnits(const UnitsPtr &importUnits, const std::
}

std::string resolvingUrl = ImporterImpl::resolvingUrl(importUnits->importSource());
Debug() << "Units -> resolvingUrl: " << resolvingUrl;

if (encounteredRelatedError) {
auto issue = Issue::IssueImpl::create();
Expand Down Expand Up @@ -653,6 +662,7 @@ bool Importer::resolveImports(ModelPtr &model, const std::string &basePath)

clearImports(model);
auto normalisedBasePath = normalisePath(basePath);
Debug() << "normalisedBasePath: " << normalisedBasePath;

for (const UnitsPtr &units : getImportedUnits(model)) {
history.clear();
Expand All @@ -670,6 +680,7 @@ bool Importer::resolveImports(ModelPtr &model, const std::string &basePath)
status = false;
}
}
printImportLibrary(pFunc()->mLibrary);

return status;
}
Expand Down Expand Up @@ -842,6 +853,8 @@ ComponentPtr flattenComponent(const ComponentEntityPtr &parent, ComponentPtr &co
}

// Get list of required units from component's variables and math cn elements.
printModel(clonedImportModel);
printComponent(importedComponentCopy);
std::vector<UnitsPtr> requiredUnits = unitsUsed(clonedImportModel, importedComponentCopy);

std::vector<UnitsPtr> uniqueRequiredUnits;
Expand Down
5 changes: 5 additions & 0 deletions src/utilities.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ limitations under the License.
#include "xmldoc.h"
#include "xmlutils.h"

#include "debug.h"

namespace libcellml {

static const std::map<std::string, int> standardPrefixList = {
Expand Down Expand Up @@ -749,6 +751,9 @@ std::vector<UnitsPtr> unitsUsed(const ModelPtr &model, const ComponentConstPtr &
auto u = v->units();
if ((u != nullptr) && !isStandardUnitName(u->name()) && (model != nullptr)) {
auto modelUnits = model->units(u->name());
Debug() << "Info:";
Debug() << model->name();
Debug() << u->name();
auto availableUnits = modelUnits ? modelUnits : u;
auto requiredUnits = referencedUnits(model, availableUnits);
usedUnits.insert(usedUnits.end(), requiredUnits.begin(), requiredUnits.end());
Expand Down
25 changes: 25 additions & 0 deletions tests/importer/model_flattening.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1918,3 +1918,28 @@ TEST(ModelFlattening, modelWithCnUnitsNotDefinedInImportedComponent)
EXPECT_EQ(size_t(1), importer->errorCount());
EXPECT_EQ("The model is not fully defined.", importer->error(0)->description());
}

TEST(Experiment, periodicStimulus)
{
auto parser = libcellml::Parser::create(false);
auto model = parser->parseModel(fileContents("importer/periodicstimulus/experiments/periodic-stimulus.xml"));

EXPECT_EQ(size_t(0), parser->errorCount());

auto validator = libcellml::Validator::create();

validator->validateModel(model);

EXPECT_EQ(size_t(0), validator->issueCount());
EXPECT_TRUE(model->hasUnresolvedImports());

auto importer = libcellml::Importer::create(false);

importer->resolveImports(model, resourcePath("importer/periodicstimulus/experiments"));

EXPECT_FALSE(model->hasUnresolvedImports());

auto flattenModel = importer->flattenModel(model);

EXPECT_NE(nullptr, flattenModel);
}
9 changes: 9 additions & 0 deletions tests/resources/importer/periodicstimulus/components/INa.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<?xml version='1.0' encoding='UTF-8'?>
<model name="INa" xmlns="http://www.cellml.org/cellml/1.1#" xmlns:cellml="http://www.cellml.org/cellml/1.1#" xmlns:cmeta="http://www.cellml.org/metadata/1.0#" xmlns:xlink="http://www.w3.org/1999/xlink">
<import xlink:href="units.xml">
<units name="uA_per_cmsq" units_ref="uA_per_cmsq"/>
</import>
<component name="INa">
<variable name="INa" private_interface="out" public_interface="out" units="uA_per_cmsq"/>
</component>
</model>
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?xml version='1.0' encoding='UTF-8'?>
<model name="stimulated" xmlns="http://www.cellml.org/cellml/1.1#" xmlns:cellml="http://www.cellml.org/cellml/1.1#" xmlns:cmeta="http://www.cellml.org/metadata/1.0#" xmlns:xlink="http://www.w3.org/1999/xlink">
<import xlink:href="units.xml">
<units name="uA_per_cmsq" units_ref="uA_per_cmsq"/>
</import>
<import xlink:href="INa.xml">
<component component_ref="INa" name="INa"/>
</import>
<component name="action_potential">
<!-- <variable name="Istim" private_interface="out" public_interface="in" units="uA_per_cmsq"/>-->
<variable name="INa" private_interface="in" public_interface="out" units="uA_per_cmsq"/>
</component>
<group>
<relationship_ref relationship="encapsulation"/>
<component_ref component="action_potential">
<component_ref component="INa"/>
</component_ref>
</group>
<connection>
<map_components component_1="action_potential" component_2="INa"/>
<map_variables variable_1="INa" variable_2="INa"/>
</connection>
</model>
13 changes: 13 additions & 0 deletions tests/resources/importer/periodicstimulus/components/units.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<?xml version='1.0' encoding='UTF-8'?>
<model name="units" xmlns="http://www.cellml.org/cellml/1.1#" xmlns:cellml="http://www.cellml.org/cellml/1.1#">
<units name="cm">
<unit prefix="centi" units="metre"/>
</units>
<units name="uA">
<unit prefix="micro" units="ampere"/>
</units>
<units name="uA_per_cmsq">
<unit units="uA"/>
<unit exponent="-2" units="cm"/>
</units>
</model>
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<?xml version='1.0' encoding='UTF-8'?>
<model name="periodic_stimulus" xmlns="http://www.cellml.org/cellml/1.1#" xmlns:cellml="http://www.cellml.org/cellml/1.1#" xmlns:xlink="http://www.w3.org/1999/xlink">
<import xlink:href="../components/units.xml">
<units name="uA_per_cmsq" units_ref="uA_per_cmsq"/>
</import>
<import xlink:href="../components/stimulated.xml">
<component component_ref="action_potential" name="_model"/>
</import>
</model>
Loading

0 comments on commit c9f69b1

Please sign in to comment.