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.

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

Wishlist of functions from writing tutorials #509

Open
kerimoyle opened this issue Jan 7, 2020 · 4 comments
Open

Wishlist of functions from writing tutorials #509

kerimoyle opened this issue Jan 7, 2020 · 4 comments

Comments

@kerimoyle
Copy link
Contributor

Hi all

Here's a list of things which would have been super useful to have that I've found while writing the tutorials for our common use cases. Some of them I've implemented in utility functions that will go alongside the tutorials, but I wanted to get your thoughts on whether the same functionality could/should be included in libcellml too?

String replace for names of units within MathML statements. I've used:

void switchUnitsInMaths(std::string &maths, std::string in, std::string out)
{
    //  Note that this function will replace any and all occurrences of the "in"
    //  string within the "maths" string with the "out" string.  It's worth noting that
    //  in order to be sure that only full name matches for units are replaced, we exploit
    //  the fact that the units names in the MathML string will be in quotation marks, and include
    //  these quotation marks on either side of the in and out strings for safety.

    std::string::size_type n = 0;
    std::string in_with_quotes = "\"" + in + "\"";
    std::string out_with_quotes = "\"" + out + "\"";

    while ((n = maths.find(in_with_quotes, n)) != std::string::npos) {
        maths.replace(n, in_with_quotes.size(), out_with_quotes);
        n += out_with_quotes.size();
    }
    std::cout << "Switched units '" << in << "' for units '" << out << "'" << std::endl;
}

I'm tossing up how best to do the same for variables, as they're not neatly surrounded by quotation marks, so gets complicated when there is a variable name which is a substring of another variable name. It could be done removing whitespace and using > and < as surrounds.

Generic header/footer tags for MathML supplied by a macro/variable/function return. In every tutorial I have to retype the header, and it seems like it could be something we don't ask users to deal with each time (unless they want to do it manually).

    std::string mathHeader = "<math xmlns=\"https://www.w3.org/1998/Math/MathML\" xmlns:cellml=\"https://www.cellml.org/cellml/2.0#\">\n";
    std::string mathFooter = "</math>";

Better handling of individual equations in MathML. Am I right in thinking that we don't allow either of:

  • a + b = c + d
  • a = b = c + d

If that's true, and we can only ever deal with equations of the form a = b + c + d, then a function that splits the MathML using the <apply><eq/> tags and is callable using the LHS variable inside the <ci></ci> tags would be really useful ... It would mean that we can edit the MathML from another component after importing or parsing. At the moment it's all or nothing which seems like a waste to me?

Better interaction with generated files. Currently the #include statement in the implementation code assumes that the interface code is in a file called model.h. While there is a function in the generator where the user can change this, it's mushed in with other stuff that a user won't know about. It would be great to have a function that just changes the name of the include file without needing to know about the rest of it ... see discussion elsewhere (here and onwards).

Validation tests funky exponent units . Does it make sense to have exponents with dimensional units? Should we be checking for this?

Here endeth the novel (for now) ...

@agarny
Copy link
Contributor

agarny commented Jan 7, 2020

Re:

  • String replace for names of units within MathML statements: it might be handy for model developers, but it has nothing to do in libCellML. We might, however, want to add a thin layer on top of libCellML that would include such a method. Maybe.
  • Generic header/footer tags for MathML: I guess this one could be part of libCellML.
  • Better handling of individual equations in MathML: a + b = c + d is allowed (even though it's not currently supported by OpenCOR; see issue CellML Text view: allow anything on the LHS of an equation opencor/opencor#576), but not a = b = c + d.
  • Better interaction with generated files: I don't see the need for this, but I understand that some of you guys do, so feel free to create an issue for it.
  • Validation tests funky exponent units: just to be sure, are you talking about having something like a^3 with 3 being in, say, millivolts? If so, this is not allowed. An exponent should be dimensionless. I don't believe we say anything about this. Maybe we should? @nickerso? @MichaelClerx?

@nickerso
Copy link
Contributor

nickerso commented Jan 7, 2020

Re:

  • String replace for names of units within MathML statements: it might be handy for model developers, but it has nothing to do in libCellML. We might, however, want to add a thin layer on top of libCellML that would include such a method. Maybe.

I agree that this is something that could go into an auxiliary support/utility code that people could use which may get rolled into libCellML at some point in the future. I disagree with doing this type of thing based on string processing - should be done treating the MathML as XML and handling namespaces properly.

  • Generic header/footer tags for MathML: I guess this one could be part of libCellML.

If we default to including the MathML namespace declaration on the model element when printing, then this reduces to <mathml:math>...</mathml:math> or similar, so that would be my suggested approach if we wanted to change things.

  • Better handling of individual equations in MathML: a + b = c + d is allowed (even though it's not currently supported by OpenCOR; see issue opencor/opencor#576), but not a = b = c + d.

There is nothing in CellML itself that forbids a = b = c + d - remember = in all cases is the logical equality operator, not assignment. Simulation tools are unlikely to know what to do with such an equation, but it is perfectly valid CellML.

  • Validation tests funky exponent units: just to be sure, are you talking about having something like a^3 with 3 being in, say, millivolts? If so, this is not allowed. An exponent should be dimensionless. I don't believe we say anything about this. Maybe we should? @nickerso? @MichaelClerx?

Again, perfectly valid CellML and typically ignored by all current simulation software except JSim. I'd expect this to be an issue that would be picked up in the units "validation" that @awickens is working on this summer. An error in the mathematical model, but not an error in the CellML...

@agarny
Copy link
Contributor

agarny commented Jan 7, 2020

Agreed about a = b = c + d being allowed in MathML (rather than CellML itself). I just wish it wasn't allowed in CellML. Nevermind.

Same with funky exponent units...

@kerimoyle
Copy link
Contributor Author

It might be handy for model developers, but it has nothing to do in libCellML.

@agarny Umm ... huh? Surely model developers are a fairly big slice of the user pie? Otherwise, why have I been writing tutorials for them for the last month ... :|

I disagree with doing this type of thing based on string processing - should be done treating the MathML as XML and handling namespaces properly.

@nickerso Agree that it would be a huge amount better to have this done properly using XML instead! If CellML is all about reusability and reproducibility, then the more we support changes and adaptations of models via the library the better, no?

I'd expect this to be an issue that would be picked up in the units "validation" that @awickens is working on this summer. An error in the mathematical model, but not an error in the CellML...

Something to add to the list of "issues" ... cf validation errors

Better interaction with generated files: I don't see the need for this, but I understand that some of you guys do, so feel free to create an issue for it.

Will do, it's an easy fix :) thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants