RedlichKisterVPSSTP bug and fix

[pwcnorthrop]

RedlichKisterVPSSTP::getdlnActCoeffdlnX_diag() returns the array dlnActCoeffdlnX_diag_.

The problem was that dlnActCoeffdlnX_diag_ was never getting updated and thus always returned an array of zeros. I created a function s_update_dlnActCoeff_dlnX_diag() to, well, update dlnActCoeffdlnX_diag_ so it now works now as expected.

[speth]

Is there a difference between dlnActCoeffdlnX_diag_[k] and dlnActCoeff_dX_(k,k)? If not, maybe it would make more sense to have getdlnActCoeffdlnX_diag extract those values rather than nearly-duplicating the implementation of s_update_dlnActCoeff_dX_.

I would also like to draw your attention to #267, where it is noted that there are no tests or examples for this class. Since you are apparently using this class, do you have any examples or tests which could be incorporated into the Cantera test suite?

[pwcnorthrop]

There's a couple differences between dlnActCoeffdlnX_diag_[k] and dlnActCoeff_dX_(k,k):

1. dlnActCoeffdlnX_diag_[k] is a derivative wrt the natural log of the mole fraction, whereas dlnActCoeff_dX_(k,k) is just wrt to the mole fraction. That's minor, just a factor of X[k] difference.

2. dlnActCoeffdlnX_diag_[k] is the total derivative, while dlnActCoeff_dX_(k,k) is calculating the partial derivative. That's more difficult to work around, and I don't think can be obtained from the dlnActCoeff_dX_ matrix, which is why I just made a new function.

And yes, I have some examples which I think I could clean up and add to the test suite.

[speth]

Got it. I don't really like the amount of redundancy between the two methods, but I guess it's no worse than what's already there in the other "update" methods of RedlichKisterVPSSTP.

If you can add some examples to the test suite, that would be spectacular. At this point, new tests should be added to either the Python test suite (in interfaces/cython/cantera/test) or the GoogleTest test suite (in test/thermo) instead of the legacy test suite in the test_problems subdirectory.

[speth]

@pwcnorthrop Any possibility of getting any examples which utilize this method (or any part of RedlichKisterVPSSTP)? I can take care of integrating them into the test suite if you're not likely to have time. Otherwise, I can merge this patch, but that still leaves the class in the precarious position of having no tests.

[pwcnorthrop]

@speth Sorry, I had got caught up on other things and forgot about setting this up. Anyway, I've added a test to the test/thermo/ directory, as well another test case in test_problems, which also serves as an example case.

[speth]

Thanks, this looks promising. Two hopefully small requests:

• Can you rework the commits so that this is just a sequence of commits on a branch, so that it can be rebased onto master? Right now, you have it so that the merge commit with master is actually where the tests are introduced, which is a bit convoluted, although I can probably disentangle this if it gives you trouble.
• Can you put all of the tests into the GoogleTest suite? We're trying to (eventually) eliminate the homegrown testing infrastructure of the test_problems directory, so new tests are all in the GoogleTest suite, and old ones are being slowly migrated in that direction.

[pwcnorthrop]

Ok, sorry this has been a mess. This is my first time using git and github, so it's been a bit of a (admittedly slow) learning process. I couldn't figure out how to revert cleanly back to before I merged the master, so I ended up committing/pushing changes that simply removed the test case. And the final push should just be the addition of the test case in the test directory only, and an example case into samples/cxx (really what I had put into test_problems last week).

Thanks for your patience.