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

Autogenerated clib core objects #1842

Merged
merged 24 commits into from
Feb 2, 2025
Merged

Conversation

ischoegl
Copy link
Member

@ischoegl ischoegl commented Jan 24, 2025

Changes proposed in this pull request

  • Add all Cantera "core" objects to experimental CLib
  • APIs for Solution, ThermoPhase, Kinetics, Transport and MultiPhase have reached parity with traditional CLib
  • Additional unit tests are added (go beyond traditional CLib test suite)
  • Some differences of method names compared to traditional CLib API are rolled back - pre-existing inconsistencies are left to be addressed at a later point
  • Logic in _orchestrate.py is simplified

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

Addresses Cantera/enhancements#220

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

Copy link

codecov bot commented Jan 26, 2025

Codecov Report

Attention: Patch coverage is 16.66667% with 5 lines in your changes missing coverage. Please review.

Project coverage is 74.42%. Comparing base (573e43c) to head (cca4a12).
Report is 41 commits behind head on main.

Files with missing lines Patch % Lines
src/equil/MultiPhase.cpp 25.00% 3 Missing ⚠️
src/base/ctexceptions.cpp 0.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1842      +/-   ##
==========================================
- Coverage   74.43%   74.42%   -0.01%     
==========================================
  Files         382      382              
  Lines       53492    53498       +6     
  Branches     9055     9056       +1     
==========================================
+ Hits        39816    39817       +1     
- Misses      10609    10613       +4     
- Partials     3067     3068       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ischoegl ischoegl force-pushed the sourcegen-clib-base branch 2 times, most recently from 30bb2b7 to b51277c Compare January 27, 2025 03:57
@ischoegl ischoegl changed the title Autogenerated clib base objects Autogenerated clib core objects Jan 27, 2025
@ischoegl ischoegl marked this pull request as ready for review January 27, 2025 04:08
@ischoegl ischoegl requested a review from a team January 27, 2025 04:08
@ischoegl ischoegl force-pushed the sourcegen-clib-base branch from b51277c to ef63709 Compare January 27, 2025 13:28
Copy link
Member

@bryanwweber bryanwweber left a comment

Choose a reason for hiding this comment

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

A few small Python-related comments 😄

interfaces/sourcegen/sourcegen/_HeaderFileParser.py Outdated Show resolved Hide resolved
@ischoegl ischoegl force-pushed the sourcegen-clib-base branch 2 times, most recently from c4b84d3 to 50de8dc Compare January 27, 2025 14:43
@ischoegl
Copy link
Member Author

A few small Python-related comments 😄

Thanks @bryanwweber! All taken care of.

@ischoegl ischoegl requested a review from bryanwweber January 27, 2025 15:41
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.

This mostly looks good to me. Just a couple of suggestions.

src/equil/MultiPhase.cpp Outdated Show resolved Hide resolved
include/cantera/kinetics/Kinetics.h Outdated Show resolved Hide resolved
interfaces/sourcegen/sourcegen/_data/ctkin_auto.yaml Outdated Show resolved Hide resolved
@ischoegl ischoegl force-pushed the sourcegen-clib-base branch from 50de8dc to cca4a12 Compare February 1, 2025 19:44
@ischoegl
Copy link
Member Author

ischoegl commented Feb 1, 2025

@speth ... thanks for your comments, which are now addressed (I also rebased).

@ischoegl ischoegl requested a review from speth February 1, 2025 20:28
@speth speth merged commit e6f3e9d into Cantera:main Feb 2, 2025
47 of 49 checks passed
@ischoegl ischoegl deleted the sourcegen-clib-base branch February 2, 2025 02:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants