Segfault for maths in encapsulated components

[kerimoyle]

See test below:

TEST(Validator, mathInEncapsulatedComponentSegfault)
{
    libcellml::ModelPtr model = std::make_shared<libcellml::Model>();
    libcellml::ComponentPtr c1 = std::make_shared<libcellml::Component>();
    libcellml::ComponentPtr c2 = std::make_shared<libcellml::Component>();
    libcellml::ComponentPtr c3 = std::make_shared<libcellml::Component>();

    libcellml::Validator validator;

    model->setName("model");
    c1->setName("c1");
    c2->setName("c2");
    c3->setName("c3");

    model->addComponent(c1);
    model->addComponent(c2);
    c2->addComponent(c3);

    const std::string math =
        "<math xmlns=\"http://www.w3.org/1998/Math/MathML\">\n"
        "  <apply>\n"
        "    <eq/>\n"
        "    <ci>v</ci>\n"
        "    <cn cellml:units=\"dimensionless\">1</cn>\n"
        "  </apply>\n"
        "</math>\n";

    libcellml::VariablePtr v = std::make_shared<libcellml::Variable>();
    v->setName("v");
    v->setUnits("dimensionless");
    c3->addVariable(v);

    validator.validateModel(model);
    EXPECT_EQ(size_t(0), validator.errorCount());  // passes fine ... 

    c3->setMath(math);

    validator.validateModel(model);  // SEGFAULTS HERE
    EXPECT_EQ(size_t(0), validator.errorCount());
}

[hsorby]

FYI: Soon the math in a component will need to be a complete sub-document so this

"<math xmlns=\"http://www.w3.org/1998/Math/MathML\">\n"

should be this

"<math xmlns=\"http://www.w3.org/1998/Math/MathML\" xmlns:cell=\"http://www.cellml.org/cellml/2.0#\">\n"

to validate without errors.

[agarny]

Wouldn't it be sufficient to have something like:

<?xml version='1.0' encoding='UTF-8'?>
<model name="my_model" xmlns="http://www.cellml.org/cellml/1.0#" xmlns:cellml="http://www.cellml.org/cellml/2.0#">
    <component name="my_component">
        ...
        <math xmlns="http://www.w3.org/1998/Math/MathML">
            ...
            <cn cellml:units="dimensionless">1</cn>
            ...
            <cn cellml:units="my_unit">1</cn>
            ...
        </math>
    </component>
</model>

This is what we have in our CellML 1.0/1.1 models.

[hsorby]

When parsing a file then yes your example is fine but when creating a model through the API with appending math strings the math in the component will need to be a complete sub-document requiring the declaration of the cellml xml namespace (if used).

[agarny]

Hmm, not sure I like that. From an enduser's perspective, I would expect the API to generate the CellML XML namespace for me automatically (using cellml by default) , and to allow me to change that value, if I want.

This means that when adding math, I wouldn't have to worry about specifying that CellML XML namespace every time. Now, the downside of my approach is that you will have to use the same CellML XML namespace for the different math bits that I will add through the API, but that's something I would be ok with, as an enduser.

[hsorby]

There in lies the problem what is the default namespace? A namespace generator might produce something like ns1 for the namespace.

I think whichever path we take we will need to document it accurately so that endusers know exactly what is expected. I am pro enduser doing less but against black box magic.

[agarny]

What is wrong with going with cellml by default and allowing the user to change it if s/he wants? Thus, when serialising the model, libCellML will by default generate something like:

<?xml version='1.0' encoding='UTF-8'?>
<model name="my_model" xmlns="http://www.cellml.org/cellml/1.0#" xmlns:cellml="http://www.cellml.org/cellml/2.0#">
    ...
</model>

or something like this if we decide to use my_cellml_namespace as the namespace instead:

<?xml version='1.0' encoding='UTF-8'?>
<model name="my_model" xmlns="http://www.cellml.org/cellml/1.0#" xmlns:my_cellml_namespace="http://www.cellml.org/cellml/2.0#">
    ...
</model>

[hsorby]

Nothing wrong with it we just need to get agreement on how this issue is going to be dealt with.

[kerimoyle]

@hsorby is there another namespace called just "cell" or was that a typo?

"<math xmlns=\"http://www.w3.org/1998/Math/MathML\" xmlns:cell=\"http://www.cellml.org/cellml/2.0#\">\n"

... and while the discussion is good, it's missing the point of this issue ... why does including maths from the API into an encapsulated component cause a segfault? It doesn't happen if the maths is in either of the other components ... ?

[hsorby]

This segfault doesn't happen in the improved math validation that is waiting for review so it will go away once that gets merged!

[kerimoyle]

OK, great :)