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

Yaml writer bug after removing some originally duplicate reactions #1629

Closed
lavdwall opened this issue Sep 29, 2023 · 2 comments · Fixed by #1740
Closed

Yaml writer bug after removing some originally duplicate reactions #1629

lavdwall opened this issue Sep 29, 2023 · 2 comments · Fixed by #1740
Labels
bug Input Input parsing and conversion (for example, ck2yaml)

Comments

@lavdwall
Copy link
Contributor

In python: When doing mechanism reduction based on reaction rates of progress, it is possible that duplicate reactions in the original mechanism are no longer duplicate in the reduced mechanism. Those reactions can be identified and the "duplicate" property can be set to False. However, since this property change is not pushed to the underlying C++ object, when the reduced mechanism is written to a *yaml file using the YamlWriter, the originally duplicate reactions are still indicated as being duplicate, and reading the reduced mechanism gives an error.

Steps to reproduce
Taking GRI-3.0 as example, in which reaction 87 and 88 are duplicate. I will remove reaction 88 and set reaction 87 to non-duplicate, then write that "reduced" model to yaml. Importing this new reduced model gives an error.

gas = ct.Solution('gri30.yaml')
reactions = [gas.reaction(i) for i in range(gas.n_reactions) if i != 88]
reactions[87].duplicate = False
gas_reduced = ct.Solution(name="gas", thermo="ideal-gas", kinetics="gas", species=gas.species(), reactions=reactions)
gas_reduced.write_yaml('gri30_test_red.yaml')

Now read the new mechanism:
gas_reduced = ct.Solution('gri30_test_red.yaml')

Behavior

Gives the following error:

InputFileError thrown by Kinetics::checkDuplicates:
Error on line 1186 of ./gri30_test_red.yaml:
No duplicate found for declared duplicate reaction number 87 (H2O2 + OH <=> H2O + HO2)

@ischoegl
Copy link
Member

ischoegl commented Sep 29, 2023

Thanks, @lavdwall for reporting the issue. I can reproduce, where

*******************************************************************************
InputFileError thrown by Kinetics::checkDuplicates:
Error on line 1186 of ./gri30_test_red.yaml:
No duplicate found for declared duplicate reaction number 87 (H2O2 + OH <=> H2O + HO2)
|  Line |
|  1181 |   - equation: 2 OH <=> H2O + O
|  1182 |     rate-constant: {A: 35.7, b: 2.4, Ea: -8.82824e+06}
|  1183 |   - equation: HO2 + OH <=> H2O + O2
|  1184 |     rate-constant: {A: 1.45e+10, b: 0.0, Ea: -2.092e+06}
|  1185 |     duplicate: true
>  1186 >   - equation: H2O2 + OH <=> H2O + HO2
              ^
|  1187 |     rate-constant: {A: 2.0e+09, b: 0.0, Ea: 1.786568e+06}
|  1188 |     duplicate: true
|  1189 |   - equation: C + OH <=> CO + H
*******************************************************************************

@ischoegl ischoegl added bug Input Input parsing and conversion (for example, ck2yaml) labels Sep 29, 2023
@ischoegl
Copy link
Member

ischoegl commented Jun 26, 2024

On closer inspection, the duplicate flag is correctly passed to the C++ layer; however, buffered input data still hold the original value and overwrite things in Reaction::parameters. See

AnyMap Reaction::parameters(bool withInput) const
{
AnyMap out;
getParameters(out);
if (withInput) {
out.update(input);
}

Reversing the order of operations (first write input, then overwrite with actual parameter values) breaks a surprising amount of logic.

speth added a commit to speth/cantera that referenced this issue Jul 26, 2024
speth added a commit to speth/cantera that referenced this issue Jul 26, 2024
speth added a commit to speth/cantera that referenced this issue Jul 27, 2024
speth added a commit to speth/cantera that referenced this issue Jul 27, 2024
speth added a commit to speth/cantera that referenced this issue Jul 27, 2024
pjsingal pushed a commit to pjsingal/cantera that referenced this issue Sep 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Input Input parsing and conversion (for example, ck2yaml)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants