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

Link C++ and Python SolutionArray implementations #1426

Merged
merged 15 commits into from
Mar 16, 2023

Conversation

ischoegl
Copy link
Member

@ischoegl ischoegl commented Jan 20, 2023

Changes proposed in this pull request

Follow-up to #1385.

  • Expose C++ core SolutionArray to Python (as SolutionArrayBase)
  • Load YAML and HDF SolutionArray formats from Python
  • Add support for additional extra types in C++ SolutionArray that are already available in Python, e.g. entries with types string, long int, vector<double> and vector<long int> (internal handling leverages AnyValue)
  • Link C++ and Python versions of SolutionArray
  • Deprecate Python h5py support and associated dedicated functions; continue support for pre-existing HDF format with core C++ HDF routines; disable creation of new HDF files with legacy format.

With this change, all data available for SolutionArray are stored by the C++ core object, with the Python SolutionArray serving as an external API. At the same time, all property calculations are still handled by the Python interface, although states are being updated in the C++ core.

Due to the size of the PR, some additional features are postponed for a follow-up:

  • Save/restore from CSV data (see read_csv does not work for species name with a comma #1372)
  • Backport selected SolutionArray.write_hdf capabilities for SolutionArray.save: e.g. default group names
  • Implement SolutionArray::show() with ascii pandas-style formatting.
  • Make Python SolutionArray picklable/copyable (via YAML)
  • Initial ReactorNet.save implementation for single-reactor networks
  • Allow for replacing groups in HDF via H5Ldelete see SO post, which is exposed in HighFive as unlink (see declaration)

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

Resolves Cantera/enhancements#137

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

While the way data are handled internally is completely refactored, most changes are completely under the hood, with SolutionArray.save and SolutionArray.restore being an exception.

In [1]: import cantera as ct
   ...: gas = ct.Solution("h2o2.yaml")
   ...: arr = ct.SolutionArray(gas, (5), extra={"spam": [1, 2, 3, 4, 5]})
   ...: arr.save("test.yaml", "entry", description="SolutionArray saved as YAML")
   ...: arr.save("test.h5", "entry", description="SolutionArray saved as HDF")

In [2]: a = ct.SolutionArray(gas)
   ...: a.restore("test.yaml", "entry")
Out[2]:
{'description': 'SolutionArray saved as YAML',
 'generator': 'Cantera SolutionArray',
 'cantera-version': '3.0.0a3',
 'git-commit': "'6b3079996'",
 'date': 'Wed Feb  1 07:13:13 2023'}

In [3]: b = ct.SolutionArray(gas)
   ...: b.restore("test.h5", "entry")
Out[3]:
{'cantera-version': '3.0.0a3',
 'date': 'Wed Feb  1 07:13:13 2023',
 'description': 'SolutionArray saved as HDF',
 'generator': 'Cantera SolutionArray',
 'git-commit': "'6b3079996'"}

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 Jan 20, 2023

Codecov Report

Merging #1426 (750521f) into main (b076ed9) will increase coverage by 0.26%.
The diff coverage is 80.29%.

@@            Coverage Diff             @@
##             main    #1426      +/-   ##
==========================================
+ Coverage   69.61%   69.87%   +0.26%     
==========================================
  Files         373      373              
  Lines       55846    56686     +840     
  Branches    18338    18883     +545     
==========================================
+ Hits        38875    39609     +734     
- Misses      14516    14564      +48     
- Partials     2455     2513      +58     
Impacted Files Coverage Δ
include/cantera/base/AnyMap.h 79.41% <ø> (ø)
src/oneD/Domain1D.cpp 76.21% <0.00%> (ø)
src/oneD/Boundary1D.cpp 55.41% <60.00%> (ø)
include/cantera/base/AnyMap.inl.h 53.62% <64.00%> (+1.89%) ⬆️
src/base/AnyMap.cpp 83.24% <67.50%> (-0.57%) ⬇️
interfaces/cython/cantera/onedim.py 89.38% <70.58%> (+1.24%) ⬆️
src/base/Storage.cpp 80.89% <75.11%> (+23.57%) ⬆️
src/base/SolutionArray.cpp 78.17% <77.10%> (+2.54%) ⬆️
interfaces/cython/cantera/composite.py 89.07% <96.89%> (-1.02%) ⬇️
interfaces/cython/cantera/solutionbase.pyx 89.66% <98.66%> (+3.44%) ⬆️
... and 5 more

... and 2 files with indirect coverage changes

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

@ischoegl ischoegl force-pushed the link-solution-arrays branch 4 times, most recently from e71a4fc to 7897e25 Compare January 23, 2023 21:31
@ischoegl ischoegl force-pushed the link-solution-arrays branch 16 times, most recently from 2ae1934 to 0787f94 Compare January 30, 2023 20:16
@ischoegl ischoegl force-pushed the link-solution-arrays branch 6 times, most recently from 13b2a17 to d6b0de9 Compare February 1, 2023 18:14
This is a squashed commit implementing the following:
- Make data shareable and sliceable
- Order SolutionArray extra entries
- Switch extra to AnyValue
- Use AnyValue in addExtra
- Set/get extra entries as AnyMap
- Set/get components as AnyValue
- Refine extra component handling
- Add shape information in C++
- Add additional HDF storage modes
- Access entries as 'loc' rather than 'index' (This change of
  nomenclature is inspired by pandas, where the index refers to the
  indexing column, while location refers to the row.
- Update HDF subfolder logic
- Address edge cases in HDF Storage wrapper
- Fix edge cases in SolutionArray
Files written using legacy HDF format introduced in Cantera 2.5.
All files are created based on the test suite of Cantera 2.6.

Note: legacy (h5py-based) writer removed in Cantera 3.0; format remains
readable with native (HighFive-based) HDF support.
Base class SolutionArrayBase (implemented in solythonbase.pyx) is used
as interface
Disable creation of HDF using h5py

Redirect SolutionArray.write_hdf to SolutionArray.save in order
to prevent new files with deprecated HDF format.
@ischoegl
Copy link
Member Author

@speth - thank you for the review! I believe that everything is addressed; I also rebased to #1455.

@ischoegl ischoegl requested a review from speth March 14, 2023 22:07
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 for the updates, @ischoegl. I have just a couple of remaining comments.

include/cantera/base/SolutionArray.h Outdated Show resolved Hide resolved
src/base/SolutionArray.cpp Outdated Show resolved Hide resolved
@ischoegl ischoegl requested a review from speth March 16, 2023 03:54
@ischoegl
Copy link
Member Author

@speth ... I adopted both suggestions/requests.

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 think this is good to go. I'm going to ignore the transient failure of the "coverage" builder, as we already have the coverage testing results.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

C++ Equivalent of SolutionArray
2 participants