Conversation
…misidentification Cantera's Python API, when constructing ct.Reaction from reactants/products dicts, silently re-interprets any species with net-zero stoichiometry (equal count on both sides — a spectator or surface catalyst) as a third-body collider. This mutates input_data: the equation string has the spectator's stoichiometry doubled and a spurious 'efficiencies' entry is added. The corrupted YAML cannot be round-tripped. This affects all rate types (ArrheniusRate, InterfaceArrheniusRate, StickingArrheniusRate, PlogRate, ChebyshevRate) and is reproducible on Cantera 3.1.0 and 3.2.0. A bug report has been filed with the Cantera project. Fix: replace the reactants/products dict form of ct.Reaction with the equation-string form for all non-third-body reaction types. Passing an equation string avoids the misidentification entirely. ThirdBody, Troe, and Lindemann reactions are left using the dict form because they require the third_body= keyword parameter, and their species do not appear on both sides so the bug does not affect them. A nested helper _ct_equation() is added inside to_cantera() to build the equation string from the already-computed ct_reactants / ct_products dicts. Also adds a regression test (test_reaction_to_dicts_surface_spectator_species) that verifies no 'efficiencies' key and no doubled stoichiometry appear for a SurfaceArrhenius reaction where the same surface species appears on both sides. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
defines base elements from existing list, but also includes custom elements and surface site as done in cantera1 method
Replaces the ad-hoc sum over atoms with the existing Molecule API method, and only emits the 'sites' field when a species occupies more than one site (monodentate adsorbates with sites=1 don't need the field). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…annotated file The way things are set up currently, transport notes belong in annotated output only. The previous commit removed the verbose guards, to match the ck2yaml version. Yes, we want them consistent, but for now make it so the non-annotated yaml files have no comments/notes, in any version. Instead: restore if-verbose gating in both yaml_cantera1 and yaml_cantera2, and strip transport notes from cantera_from_ck/chem.yaml after ck2yaml generates it when verbose_comments is False, so all three non-annotated files are consistent. To remove them from the ck2yaml-generated file, we can use a regex to remove 'note:' lines and their indented continuations . Avoieds yaml.safe_load + yaml.dump (which reformats the whole file) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…res cantera2 writer to ck2yaml as is done for cantera1
…haviour Two tests were written against the original (ungated) transport note behaviour and against sites=1 being emitted for monodentate species: - Rename and invert the transport note tests: note must be absent when verbose=False and present when verbose=True. - Single-site surface species should have no 'sites' key (Cantera defaults to 1); only bidentate+ species emit the field. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
These files are generated on the fly by running the mainTest functional test. Committing them bakes in machine-specific paths and database-version-dependent thermo coefficients that drift over time. The comparison test already skips gracefully when the files are absent, so CI is unaffected. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…d compares cantera2 writer to ck2yaml as is done for cantera1
Both Cantera YAML writers and the Chemkin writer were emitting a hardcoded periodic-table grab-bag (H, C, O, N, Ne, Ar, He, Si, S, F, Cl, Br, I plus D/T/CI/OI isotopes plus X) in every output file, regardless of what the model actually contained. This polluted the files and caused diffs against ck2yaml-converted reference outputs. Add ReactionModel.get_elements() that walks each species' molecule[0] atoms and returns the set of Element singletons in use. All three writers now derive their elements block from that set: built-in elements appear only when present; isotopes (D, T, CI, OI) appear only when an isotope atom is on some species; X appears only for surface models. Writer 2's is_plasma path still adds the E pseudo-element without iterating atoms. In chemkin.pyx save_chemkin, the union is computed once across all species before the surface/gas split so that the gas-only Chemkin file in a surface run still lists X (required for downstream ck2yaml conversion to recognize the surface site element). We could cache this list of elements, either updated once per save_all() call, or even just once after model initiation (since no chemistry can create an element that wasn't already there). But that is not done yet. (For simplicity) Side cleanups: drop the unused search_for_additional_elements branch in writer 2 (the new design subsumes it); replace writer 2's inline MixedModel with a real ReactionModel so .get_elements() works without duck typing; remove the module-import-time ELEMENTS_BLOCK/ELEMENTS_LINE globals in writer 1 in favor of per-call computation. Mock containers in the writer 2 tests now inherit ReactionModel for the same reason. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The destination directories under test/rmgpy/test_data/yaml_writer_data/ (cantera1, cantera2, ck2yaml) are not guaranteed to exist on a fresh checkout: we recently removed the committed golden YAML files leaving the subdirectories empty. Git does not track empty directories, so CI failed Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Motivation or Problem
The Cantera yaml output files written in RMG had some errors when loaded into Cantera.
CanteraError thrown by Phase::addSpecies: Species 'vacantX(3)' contains an undefined element 'X'.and also does not specify the sites correctly, giving and errorNumber of surface sites not balanced in reaction HOCOXX(73) <=> OX(6) + XCOH(54).Once fixes were applied to load mechanisms into Cantera without errors, the comparison file generated at the end of the RMG run revealed some differences.
Description of Changes
The "state" field was populated in each place it was missing for the rmg writer but present in ck2yaml. The species transport note had been turned off for the non-annotated yaml files, so this condition was removed to be consistent with ck2yaml. The gas and surface reactions were separated for surface mechanisms for the cantera2 method, which enabled the comparison method. The cantera1 logic was modified for use in cantera2 to specify custom elements. For surface species, "sites" was populated in cantera2 by counting the X atoms. To combat the duplication of species from Cantera's reaction definition, the equation was composed manually from reactants and products.
Testing
I was using this input file for methane partial oxidation: input.py which writes Cantera yaml for both methods. The comparison text files generated were used to identify differences between ck2yaml method and the RMG yaml writers. Claude was used to help make some tests that the missing fields were present and that generated yamls agree with the ck2yaml method. The new and existing tests seem to be working for me as far as I can tell.