-
Notifications
You must be signed in to change notification settings - Fork 39
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
Thermochemsity Parsing Refactoring Part 2: Antioch #550
Merged
pbauman
merged 65 commits into
grinsfem:master
from
pbauman:thermochem-parsing-part2-antioch
Jul 27, 2018
Merged
Thermochemsity Parsing Refactoring Part 2: Antioch #550
pbauman
merged 65 commits into
grinsfem:master
from
pbauman:thermochem-parsing-part2-antioch
Jul 27, 2018
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Use the builder since we have it instead of keeping copied code around.
Wanting to use this class elsewhere, and we need placeholder functions for now while we update interface internally.
This class originally only exposed building the Antioch wrapper, but I think we want to shift to building "raw" Antioch objects and exposing those. This will allow us to further consolidate Antioch construction to the Builder.
We don't have all the transport methods moved to the builder yet, but let's use the ones that we have.
These methods are deployed in the Antioch transport builder and we're using them now, so let's get rid of the code duplication.
These effectively got moved to the Antioch builder so they are not needed anymore.
It's shorter and the context of kinetics thermo vs. gas thermo is not relevant here.
We're going to move to this completely in subsequent commits, but this commit now will require a version of Antioch with libantioch/antioch#259 merged to master. (We'll identify the Git hash in subsequent README update).
Since we moved to Antioch::IdealGasThermo, we don't need to cache that silly evaluator class anymore.
And migrate all it's construction to the builder so that the builder and build everything and we can move away from any of the Antioch parsing being contained in the Antioch wrapper objects.
This class will go away at some point, but need to support until we move to an new thermochemistry+transport interface internally.
This way we don't have to do specializations for every Thermo and KineticsCurveFit pair when we go to instantiate more CurveFit models.
This is going to be the public method one uses to get the species names out from Antioch. We will subsequently add the logic for figuring out the parser type, etc. Also migrated the MaterialsParsing::parse_chemical_species as a protected member of AntiochMixtureBuilderBase. We'll use this method in the interim but it should (hopefully) eventually go away. Regardless, we don't want any other users using it.
This private helper function will tell us the Antioch::ParsingType based on the suffix of the file given to the chemical_data input option. The convention we adopt is .dat ==> ASCII, .xml ==> XML, and .chemkin ==> CHEMKIN
We will do a pass using ChemKinParser later
Was missing the transport data for the last 4 species so add it. The data was pulled from Cantera's GRI3 XML file.
Now we differentiate the parsing of the transport mixture data based on the parser type. In particular, we should now be able to grab the pure species data from the XML directly. We still support the ASCII format. We can't do anything about the non-pure species transport models since the Antioch XML parser doesn't support those (see libantioch/antioch#256)
Some species had data different than the GRI3 XML file and looked like bad copy/pasta, so I updated values to match GRI3 XML.
So we can reuse it later.
We were supposed to be using a template parameter there. Didn't come up because we've only been doing CEA to this point.
Currently only for mixture averaged transport models. This now enables us to leverage Antioch's XML parser to get the thermo data from the XML file directly when we're using XML.
Just based off of the existing 2 species regression, but using ideal_gas instead of stat_mech for the thermo model. Also acts to check parsing of data from the XML file + blottner data.
The AntiochMixtureBuilderBase::get_thermo_type should correctly handle this for us now with the way the parsing scheme is setup.
I don't know how we were getting away with what was effectively partial template specialization. Cleared out the specialization on CEACurveFit and replaced with already-existing-why-didn't-compiler- complain-about-missing template parameter for the kinetics thermo curve fit.
Same thing that we did in a435b60 to air.xml. Had to update the regression test for the reacting_low_mach_cantera test.
The thermo data in the XML is the same as the Antioch defaults so this is seamless to the user, but underneath it should be parsing from the XML file now instead of the Antioch default ascii files.
Job GRINS-devel on 004f77e : invalidated by @pbauman Updated Antioch version on femputer |
Now that we parse species names away from the GRINS input file using Antioch parsers, we broke that unit testing without Antioch. This restructures the test so that 1. We still do the variable testing and 2. We also still test the arbitrary naming of the variable.
These all run for me on 1--16 processors now.
That's why we have CI kids. Pushed tolerance tweaks for parallel runs and updated unit test that became stale when optdeps are disabled. |
pbauman
added a commit
to pbauman/grins
that referenced
this pull request
Aug 5, 2018
This should've been part of grinsfem#550 but was overlooked.
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Monster PR, but part 2 cousin of #549. This updates the Antioch side. This PR is basically a massive overhaul of the internals of parsing data for Antioch. The code changes have left placeholders for inserting the Anitoch ChemKin parser, but I haven't dropped those in yet. This does add full support for Antioch's XML parser, including not just kinetics, but thermo and transport as well. As such, this PR will push the minimum Antioch version up to libantioch/antioch@e17822d, as reflected in the README and CHANGES file. We also do not need to explicitly set the species names in the GRINS input file when using XML anymore, we parse it just as we do with Cantera. We do still require it for the ASCII parsing. Finally, the type of curve fit will be deduced based on the input file.
Backward compatibility has been maintained (and tested, didn't update test input files until the end), but didn't add new features to old style so you need to switch (but it's easy).
We determine the Antioch parsing type based on the suffix of the file specified in the new
chemical_data
option:.dat
is ASCII,.xml
is XML, and.chemkin
is CHEMKIN (which isn't implemented yet). For ASCII, you'll still have to set the species names and all the other data.The new preferred input file format for Antioch specification is identical to Cantera when using XML except
thermo_model
toideal_gas
(this is the same model as Cantera) orstat_mech
.transport_model
and associated entries.So, the new format for using XML input (that includes kinetics, thermo, transport in one file) is
For Blottner or Sutherland, you still need the data in either raw txt file or just let it default to Antioch if the species are there (which is what will happen above). For
kinetics_theory
that data is parsed from the XML file.One should now take the above input and change
antioch
tocantera
and run with Cantera seamlessly (where Cantera would then be using the transport model specified in the XML file).@bboutkov @klbudzin This should make your lives quite a bit easier.
I'm pretty happy with this except the
kinetics_theory
model doesn't work like Cantera does. But that's a separate issue. (I get divergence when starting from time 0 in the ozone_flame exampling after a few dozen time steps, and I can't get convergence when restarting from the same file we restart Cantera from to get steady state ozone flame example, dies after 4 or 5 Newton steps. @klbudzin I'm going to want your help in getting to the bottom of that.)But with this PR we get quite a bit more functionality out of Antioch and it will make it easier to test, etc.