Remove American spellings from prefixes and units (deka, meter, liter)

[kerimoyle]

I would suggest we start supporting both - especially as we've currently got "deka" in the spec, while "deca" seems to be the standard

Originally posted by @MichaelClerx in #304 (comment)

I've changed a couple of files to see what the effects of this would be. Since we can duplicate enumerated values, and since it makes sense to use those values as something meaningful (the log of the prefix value), both the "deca" and "deka" prefixes now have the same value (1). This means that reverse lookups like the prefixToString function will only ever return "deca" (and I would have thought that the map condition of unique keys is violated, but somehow it still runs fine ...?).

std::map<Prefix, const std::string> prefixToString =
{
    {Prefix::ATTO, "atto"},
    {Prefix::CENTI, "centi"},
    {Prefix::DECA, "deca"},
    {Prefix::DECI, "deci"},
    {Prefix::DEKA, "deka"},  // Added to support "deka" and "deca" as both are in common use
    {Prefix::EXA, "exa"},
    {Prefix::FEMTO, "femto"},
... etc ... 

The consequence of this is that:

• specifying a unit with a prefix via the Prefix enum will end up with a printed string of "deca", even if Prefix::DEKA is used.
• specifying a unit with a prefix of a string "deka" or "deca" will have this preserved in their printed units.

TEST(Coverage, prefixToString) {
    libcellml::Model m;
    libcellml::Printer printer;
  std::vector<std::string> prefix_str =
        {"atto",
         "centi",
         "deca",
         "deci",
         "deka",  // "deka" will be turned into "deca" on conversion through the enum ...
         "exa",
   ... etc ...
        };
    std::vector<libcellml::Prefix> prefix_enum =
        {libcellml::Prefix::ATTO,
         libcellml::Prefix::CENTI,
         libcellml::Prefix::DECA,
         libcellml::Prefix::DECI,
         libcellml::Prefix::DEKA,
         libcellml::Prefix::EXA,
 ... etc ... 
        };
    for (std::vector<std::string>::size_type i = 0; i != prefix_str.size(); ++i) {
        std::string prefix = prefix_str[i];
        libcellml::UnitsPtr u = std::make_shared<libcellml::Units>();
        u->setName("abcdefg");
        u->addUnit("empty", prefix_enum[i]);   // <<<<< This one fails, prefix added as enum
        // u->addUnit("empty", prefix);     // <<<<<< This one passes, prefix added as string is maintained
        m.addUnits(u);

        const std::string a = printer.printModel(m);
        std::size_t found = a.find(prefix);
        EXPECT_NE(std::string::npos, found);
        m.removeAllUnits();
    }
}

So ... my question is do we want to convert occurences of "deka" into "deca", or do we want to genuinely support both independently? And if the latter, is it possible to do and maintain sensible values for the enums?

See this commit and the tweaked one after it.

[nickerso]

I don't think we want to be supporting multiple spellings of a single prefix. We should go with the standard deca and update the CellML1to2 XSL to change any occurrence of deka as part of the translation.

[hsorby]

Totally agree.

[MichaelClerx]

I don't understand why we can't have both? One could be the proper one, the other a pointer/alias/synonym. No harm done?
I can imagine developers working with both CellML versions (because CellML 1.1 will be around for another decade, at least), cursing us for this idiosyncrasy.

What is the rationale for not having both? Why not Deka because we've maintained that's the proper one since 2001, Deca because we acknowledge that that was a mistake?

It's like, one line of code and 2 lines of tests?

[hsorby]

Having duplication of the same prefix doesn't make sense to me. If it is a mistake to use Deka then we should just put our hand up and have it correct going forward.

Also allowing the duplication would make enumerations a complete pain and possibly force not having to use them.

[MichaelClerx]

How would it mess up enums? It's a one-way street this: deka maps to deca, deca maps to 10, 10 maps to deca. That's the extent of it as far as I can see?

Either way, this:

Also allowing the duplication would make enumerations a complete pain and possibly force not having to use them.

is not an argument. CellML is for people who want to exchange models, not for libcellml implementers. If we need a few extra lines in libcellml (or any other libraries) to make life easier for CellML users then that is absolutely fine. That's added value. Annoying details like the exact spelling of deca/deka are precisely the sort of thing I'd want a library to deal with for me

[hsorby]

If Deka isn't the standard prefix then we shouldn't persist with it. We can easily add a transform in the xslt to move Deka to Deca. Deka appears to me to be the American spelling of the prefix. I think the spec. should be changed to use Deca and then libCellML would have no choice!

[nickerso]

deka maps to deca, deca maps to 10, 10 maps to deca.

I think this is the key - no matter which prefix is used in a model, it will always be serialised as deca, which seems more confusing than helpful.

But this is really a spec issue not a libCellML issue. Bumping to the spec issues and probably best for that discussion to resolve before deciding what changes to make in libCellML implementation.

[nickerso]

please comment on cellml/cellml-specification#21 if you have an opinion on this...

[kerimoyle]

According to discussion on the specifications thread will update the code soon. Have changed the title of this issue to follow it.

[MichaelClerx]

OK, so the spec now says SI only.
Is there anybody except from me who thinks it's a user-friendly idea to allow the youessian spellings as input but never output them?
From a user perspective, rather than a coders perspective :-)

Otherwise this seems to have been fixed in #351 ?

[kerimoyle]

We had big discussions about this at the time ... consensus was to be consistent with removing US spelling throughout as opposed to changing between inputs->outputs. If users want to use youessian (new word for me, love it) spellings then they still can just by making an alias:

auto meter = Units::create("meter");
meter->addUnit("metre");

[MichaelClerx]

Thanks!