Fixes ConstDensityThermo::standardConcentration()

[decaluwe]

Fixes #458 .

Changes proposed in this pull request:
-ConstDensityThermo::standardConcentration for species k is calculated as
density()/molecularWeight(k). Was previously calculated (incorrectly) as
molarDensity().

• Note that any species with a molecularWeight of zero (i.e. a vacancy) will
  cause problems, and therefore should be avoided for this phase.
  -For this reason, sofc-text.xml also had to be changed and models the YSZ
  electrolyte as IdealSolidSolution, rather than as incompressible_solid.

[decaluwe]

Alright, can somebody explain the codecov/patch fail to me, and what I need to do in order to fix it?

Thanks.

[bryanwweber]

@decaluwe The comment isn't the most useful is it... if you dig into the report here: https://codecov.io/gh/Cantera/cantera/pull/490/tree/src/thermo you can see that the "green dot" column (which represents number of lines that were run in the tests) decreased by 11 in the constDensityThermo.cpp file and the "red dot" column (number of lines that weren't run in the tests) increased by 11. However, the IdealSolidSolnPhase.cpp file gained 9 lines of coverage.

I suspect this has to do with you changing the thermo model in sofc-test.xml from "Incompressible" to "IdealSolidSolution"... basically, rather than running the lines in constDensityThermo.cpp, now it runs the lines in IdealSolidSolnPhase.cpp. You could consider adding a test for the code in constDensityThermo.cpp, but whether that's worth it depends on whether this class will be staying around long-term, or if this is just a temporary fix until something better is available.

The other random failures in the equil code should be fixed by 1eebd3e that @speth committed last night

[decaluwe]

Thanks, @bryanwweber - I did look at the report details, but still didn't know how to read it! 🙄

The idealSolidSolution model is the correct model for the sofc-test.xml file. We fixed the `constandDensityThermo::standardConc' calculation, but I have doubts about the long-term utility of this model, in general (I think this is detailed in the issue #458 discussion).

The assumption of a constant mass density for a non-stoichiometric substance seems... odd. Even worse, it is specified at the cti layer by the phase model name incompressible_solid, which is incredibly misleading (since that would be generally understood as constant molar density). I'd be fine deprecating the class, long term, but think in the short term the incompressible_solid call from the cti level should have a different behavior.

Regardless, I'll fetch Ray's latest commit, rebase, and re-push to my repo.

Thanks.

[speth]

I'm not concerned about the changes in code coverage, since they are all fully explained at this point. I would generally regard the code coverage checks as informational rather than a requirement for merging a PR.

There's no need to rebase onto the current master, since that will happen automatically when we merge the PR.

My only minor suggestions are:

• fix the spelling of molarDensity in the commit message (instead of molarDemsity)
• Get rid of the spurious indentation changes in the XML file (makes it hard to see what has actually changed when looking at the diff)

[decaluwe]

Hey, man, spelling is a highly subjective enterprise, sometimes... 🙄

[decaluwe]

Okay, should be good to go.

[bryanwweber]

@decaluwe I agree, the report pages are hard to read, especially the default one it links you to from the PR. I find I have to click around to about 10 different pages before I find what I'm looking for... Ah well, as @speth said, its useful as an informational point (and anyway, having full coverage doesn't mean that you've found all the bugs)