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

Update read_reaction_set_data API #261

Merged
merged 3 commits into from
Jul 10, 2018

Conversation

pbauman
Copy link
Member

@pbauman pbauman commented Jul 10, 2018

This adds a new overload of read_reaction_set_data that takes an existing pointer to a ParserBase object. Really, we just want to do #260, but I need this right away so that I can leverage the mixture specification in the XML file (which requires the constructor from #254 and is not exposed to the other read_reaction_set_data methods). So with this PR, I can construct the XMLParser object myself and just pass to this method. Added unit test of the function that also tests xml file with multiple species and reaction sections in it.

I'd like to merge quickly after a little GRINS testing.

@pbauman
Copy link
Member Author

pbauman commented Jul 10, 2018

GRINS is good with this PR.

@@ -80,6 +80,7 @@ O 16.00000 1.5420000000e7 1.5 0
O+ 15.99945 9.7560000000e7 1.5 1
O2 32.00000 0.000000000000 2.5 0
O2+ 31.99945 3.6370000000e7 2.5 1
O3 48.00000 3.0104166666e6 3.0 0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this also from http://cccbdb.nist.gov/hf0K.asp ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct. I should've included in the commit log that the comment exactly as stated is still correct. I'll force push an amendment to the commit log.

@roystgnr
Copy link
Contributor

👍 now if you're still consistent with that comment; 👍 after you cite a source otherwise.

This one takes a pointer to a ParserBase object and then does
the read. Slight refactoring of existing function to reuse existing
capability. Really, this should just all be in the ParserBase
and XMLParser classes, but until we address libantioch#260, we're stuck
with this simple change/(gross) API extension.
@pbauman
Copy link
Member Author

pbauman commented Jul 10, 2018

lol, just force pushed a small API update to my new API. I'd copy-pasted, but the filename argument in the new function was unnecessary/superfluous. Not sure why compiler didn't throw unused-variable warning.

We'll use this in a forthcoming unit test. The comment in the file
regarding the source of the formation enthalpy still exactly applies
to this file after this commit. Namely, the O3 formation enthalpy
was sourced from http://cccbdb.nist.gov/hf0K.asp
Also, this explicitly tests parsing an XML file with multiple
species and reaction sections within.
@pbauman
Copy link
Member Author

pbauman commented Jul 10, 2018

OK, force pushed again with amended commit log on the Antioch default data.

@pbauman pbauman merged commit 7cebf19 into libantioch:master Jul 10, 2018
pbauman added a commit to pbauman/grins that referenced this pull request Jul 10, 2018
In reality, both formats are using XML to parse the kinetics, but
the ASCII format requires an additional flag for the filename for
the kinetics data whereas XML it will all be stashed in one place.

This commit is also leveraging a new Antioch API that was part of
libantioch/antioch#261. Thus, the minimum Antioch hash that will
be required after this commit is  libantioch/antioch@7cebf19.

We'll work on ChemKinParser support later.
@pbauman pbauman deleted the read-reaction-data branch July 10, 2018 22:14
pbauman added a commit to pbauman/grins that referenced this pull request Jul 11, 2018
In reality, both formats are using XML to parse the kinetics, but
the ASCII format requires an additional flag for the filename for
the kinetics data whereas XML it will all be stashed in one place.

This commit is also leveraging a new Antioch API that was part of
libantioch/antioch#261. Thus, the minimum Antioch hash that will
be required after this commit is  libantioch/antioch@7cebf19.

We'll work on ChemKinParser support later.
pbauman added a commit to pbauman/grins that referenced this pull request Jul 11, 2018
In reality, both formats are using XML to parse the kinetics, but
the ASCII format requires an additional flag for the filename for
the kinetics data whereas XML it will all be stashed in one place.

This commit is also leveraging a new Antioch API that was part of
libantioch/antioch#261. Thus, the minimum Antioch hash that will
be required after this commit is  libantioch/antioch@7cebf19.

We'll work on ChemKinParser support later.
pbauman added a commit to pbauman/grins that referenced this pull request Jul 19, 2018
In reality, both formats are using XML to parse the kinetics, but
the ASCII format requires an additional flag for the filename for
the kinetics data whereas XML it will all be stashed in one place.

This commit is also leveraging a new Antioch API that was part of
libantioch/antioch#261. Thus, the minimum Antioch hash that will
be required after this commit is  libantioch/antioch@7cebf19.

We'll work on ChemKinParser support later.
pbauman added a commit to pbauman/grins that referenced this pull request Jul 19, 2018
In reality, both formats are using XML to parse the kinetics, but
the ASCII format requires an additional flag for the filename for
the kinetics data whereas XML it will all be stashed in one place.

This commit is also leveraging a new Antioch API that was part of
libantioch/antioch#261. Thus, the minimum Antioch hash that will
be required after this commit is  libantioch/antioch@7cebf19.

We'll work on ChemKinParser support later.
pbauman added a commit to pbauman/grins that referenced this pull request Jul 19, 2018
In reality, both formats are using XML to parse the kinetics, but
the ASCII format requires an additional flag for the filename for
the kinetics data whereas XML it will all be stashed in one place.

This commit is also leveraging a new Antioch API that was part of
libantioch/antioch#261. Thus, the minimum Antioch hash that will
be required after this commit is  libantioch/antioch@7cebf19.

We'll work on ChemKinParser support later.
pbauman added a commit to pbauman/grins that referenced this pull request Jul 25, 2018
In reality, both formats are using XML to parse the kinetics, but
the ASCII format requires an additional flag for the filename for
the kinetics data whereas XML it will all be stashed in one place.

This commit is also leveraging a new Antioch API that was part of
libantioch/antioch#261. Thus, the minimum Antioch hash that will
be required after this commit is  libantioch/antioch@7cebf19.

We'll work on ChemKinParser support later.
pbauman added a commit to pbauman/grins that referenced this pull request Jul 25, 2018
In reality, both formats are using XML to parse the kinetics, but
the ASCII format requires an additional flag for the filename for
the kinetics data whereas XML it will all be stashed in one place.

This commit is also leveraging a new Antioch API that was part of
libantioch/antioch#261. Thus, the minimum Antioch hash that will
be required after this commit is  libantioch/antioch@7cebf19.

We'll work on ChemKinParser support later.
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

Successfully merging this pull request may close these issues.

2 participants