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

Fix serialization of phases with reactions from multiple YAML sections #1552

Merged
merged 1 commit into from
Jul 27, 2023

Conversation

ischoegl
Copy link
Member

@ischoegl ischoegl commented Jul 23, 2023

Changes proposed in this pull request

Information on reactions in the phase input section is kept in user data, but cannot be used for the serialization, as there is no bookkeeping of where reactions originated. As it is not essential to retain this information, the user input is deleted.

If applicable, fill in the issue number this pull request is fixing

Closes #1522

If applicable, provide an example illustrating new features this pull request is introducing

See example provided in #1522.

Checklist

  • The pull request includes a clear description of this code change
  • Commit messages have short titles and reference relevant issues
  • Build passes (scons build & scons test) and unit tests address code coverage
  • Style & formatting of contributed code follows contributing guidelines
  • The pull request is ready for review

Information on reactions in the phase input section is kept in
user data, but cannot be used for the serialization, as there
is no bookkeeping of where reactions originated. As it is not
essential to retain this information, the user input is deleted.
@ischoegl ischoegl added the Input Input parsing and conversion (for example, ck2yaml) label Jul 23, 2023
@ischoegl ischoegl marked this pull request as ready for review July 23, 2023 02:55
@codecov
Copy link

codecov bot commented Jul 23, 2023

Codecov Report

Merging #1552 (c4e1fe3) into main (5f1014c) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #1552   +/-   ##
=======================================
  Coverage   70.46%   70.47%           
=======================================
  Files         379      379           
  Lines       59091    59093    +2     
  Branches    21228    21230    +2     
=======================================
+ Hits        41641    41643    +2     
  Misses      14369    14369           
  Partials     3081     3081           
Impacted Files Coverage Δ
src/base/Solution.cpp 79.56% <100.00%> (+0.22%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@ischoegl ischoegl requested a review from a team July 23, 2023 03:23
@speth speth changed the title Fix 1522 Fix serialization of phases with reactions from multiple YAML sections Jul 27, 2023
Copy link
Member

@speth speth left a comment

Choose a reason for hiding this comment

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

Works for me.

@speth speth merged commit 7fd42ac into Cantera:main Jul 27, 2023
39 of 40 checks passed
@ischoegl ischoegl deleted the fix-1522 branch July 28, 2023 18:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Input Input parsing and conversion (for example, ck2yaml)
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

write_yaml produces inconsistant reactions sections
2 participants