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

Tweak SolutionArray IO #1458

Merged
merged 7 commits into from
Mar 21, 2023
Merged

Tweak SolutionArray IO #1458

merged 7 commits into from
Mar 21, 2023

Conversation

ischoegl
Copy link
Member

@ischoegl ischoegl commented Mar 20, 2023

Changes proposed in this pull request

This PR takes introduces minor improvements to the API after the merge of #1426

  • Protect written data / enable overwriting of existing data
  • Deprecate loglevel in Sim1D::save/restore
  • Remove unnecessary output for SolutionArray::save

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

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

@codecov
Copy link

codecov bot commented Mar 20, 2023

Codecov Report

Merging #1458 (45d29c7) into main (3cbe8aa) will decrease coverage by 0.05%.
The diff coverage is 46.66%.

@@            Coverage Diff             @@
##             main    #1458      +/-   ##
==========================================
- Coverage   69.91%   69.86%   -0.05%     
==========================================
  Files         377      377              
  Lines       57262    57298      +36     
  Branches    19151    19164      +13     
==========================================
- Hits        40036    40033       -3     
- Misses      14676    14712      +36     
- Partials     2550     2553       +3     
Impacted Files Coverage Δ
include/cantera/base/SolutionArray.h 94.44% <ø> (ø)
include/cantera/oneD/Domain1D.h 79.23% <0.00%> (ø)
include/cantera/oneD/Sim1D.h 66.66% <ø> (ø)
include/cantera/oneD/StFlow.h 100.00% <ø> (ø)
src/base/Storage.cpp 79.43% <12.50%> (-1.46%) ⬇️
src/oneD/Sim1D.cpp 70.90% <33.33%> (-2.33%) ⬇️
src/base/SolutionArray.cpp 76.75% <36.66%> (-1.42%) ⬇️
src/clib/ct.cpp 18.66% <50.00%> (ø)
src/oneD/Boundary1D.cpp 55.39% <50.00%> (ø)
src/oneD/Domain1D.cpp 72.00% <50.00%> (+0.40%) ⬆️
... and 7 more

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

@ischoegl ischoegl marked this pull request as ready for review March 20, 2023 17:37
@ischoegl ischoegl changed the title Tweak SolutionArray Tweak SolutionArray IO Mar 20, 2023
@ischoegl ischoegl requested a review from a team March 21, 2023 00:32
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.

Thanks, @ischoegl. I'm definitely pleased to see the loglevel flag go away in these instances. I just had one naming suggestion, and a point of clarification on checkGroup, which may suggest some additional documentation for that method.

include/cantera/base/Storage.h Outdated Show resolved Hide resolved
test/general/test_serialization.cpp Show resolved Hide resolved
@ischoegl
Copy link
Member Author

@speth and @bryanwweber ... thanks for the reviews! I believe all concerns are addressed.

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.

Thanks, @ischoegl. This looks good to me.

@ischoegl ischoegl merged commit 8b5a0e9 into Cantera:main Mar 21, 2023
@ischoegl ischoegl deleted the tweak-SolutionArray branch March 21, 2023 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

3 participants