libantioch / antioch

C++ Chemical Kinetics, Thermodynaimics, and Transport Library
https://libantioch.github.io/
Other
23 stars 17 forks source link

xml_parser GRI30 compatible #212

Closed SylvainPlessis closed 8 years ago

SylvainPlessis commented 8 years ago

not tested yet, it should work.

No test yet, but as it's starting to get a little late here, I'm putting it here anyway, so it's at least testable.

So far I've only checked that it doesn't break anything (at least on my computer, let's see what Travis thinks).

Feel free to add the test if you have the time/will.

Edit: And of course, I've forgotten to link it to the issue.

This is the answer to issue #211

SylvainPlessis commented 8 years ago

tested and approuved by my computer. Basically, all the falloff reactions in the file _test/input_files/testparsing.xml are doubled to fit the GRI way of falloff, and the test accordingly updated.

Good night!

roystgnr commented 8 years ago

Some of these tests are actually failing under the hood, but the failures are being masked because if(error > tol) scream_and_die(); is a no-op for error==NaN.

Reactions 0028bis, 0029bis, 0030bis, 0031bis, 0032bis, 0033bis, and 0034bis are all outputting NaN because alpha, T1, and T3 have been left set to 0, and T2 has been left set to 1e50.

Reaction 0035 is failing, but in that case I'm not sure why.

I can set up a PR that reproduces the failures if it would help.

roystgnr commented 8 years ago

Looks like those bis reactions are returning parser->Troe() == false, so they're not querying the Troe parameters in the first place.

roystgnr commented 8 years ago

And in turn that's because rate_constant() is looking for a Troe subsection but isn't looking for a falloff subsection of type Troe.

roystgnr commented 8 years ago

Merging and rebasing on #228 fixes all the bis reactions for me. Now to look into 0035.

roystgnr commented 8 years ago

0035 is a photochemistry problem, apparently not a parsing problem.

roystgnr commented 8 years ago

And 0035 is fixed for me in #229. I'm loving the new regression test; thanks @SylvainPlessis