Flame restart and initial guess examples and fix issue #1288 #1293

rwest · 2022-05-26T19:15:06Z

Changes proposed in this pull request

Allow set_initial_guess to receive a Pandas DataFrame object, as documented.
Provide an example file with lots of examples of ways to save and load flame solutions and initial guesses

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

Closes #1288

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

Most of the pull request is providing examples (including one of the fixed bug).

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

ischoegl

@rwest ... thanks for looking into this! Restarts and improved file IO from SolutionArray (and FlameBase) were among the first things I implemented when I started contributing for 2.5.1 (I had used restarts from HDF extensively in the past). The pandas issue slipped through, which should have been secured by unit tests (which were never added). Fwiw, pandas itself was just a vehicle to write an earlier HDF version that never made it into a release, but it was kept around ...

While this PR may not be fully developed (some catch blocks look like this is still work in progress), I added a couple of comments. Also, several of your tests read like unit tests ... let me know if you have questions ...

interfaces/cython/cantera/examples/onedim/flame_initial_guess.py

codecov · 2022-05-31T13:13:08Z

Codecov Report

Merging #1293 (ef32e50) into main (8fbb4bc) will increase coverage by 0.00%.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1293   +/-   ##
=======================================
  Coverage   67.18%   67.19%           
=======================================
  Files         314      314           
  Lines       41904    41905    +1     
  Branches    16863    16862    -1     
=======================================
+ Hits        28155    28156    +1     
  Misses      11518    11518           
  Partials     2231     2231

Impacted Files	Coverage Δ
include/cantera/base/Solution.h	`93.33% <0.00%> (ø)`
samples/cxx/flamespeed/flamespeed.cpp	`91.89% <0.00%> (ø)`
samples/cxx/openmp_ignition/openmp_ignition.cpp	`84.84% <0.00%> (ø)`
src/base/Solution.cpp	`81.50% <0.00%> (+0.12%)`	⬆️

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

rwest · 2022-05-31T13:15:41Z

Thanks for the comments @ischoegl. I think I've addressed them (and tidied some other things up on the rebase).

ischoegl

@rwest ... thank you for updating. To follow up on my initial comments, I have a couple of more substantive suggestions.

I don't think that the example needs to check / demonstrate equivalence of restored solutions, as there are already unit tests for that purpose (I linked those that do exist - pandas is the main one that is missing). On the other hand, the pruning checks are very informative - I think it would be nice to just play around with tolerances, pruning, and refine criteria as these are not necessarily well understood by users, while giving ample opportunity to demonstrate file I/O.

Fwiw, there's another relevant example that is a little more advanced - see diffusion_flame_batch.py. I think a simpler example like the one you are proposing here is a great addition.

interfaces/cython/cantera/examples/onedim/flame_initial_guess.py

Allows you to pass a pandas DataFrame as the data argument to a flame's set_initial_guess method. Closes Cantera#1288

Examples of different ways to do this, saving via yaml, HDF5 and CSV, then loading either as a restart or as an initial guess, including with some modifications to the data (eg. fewer grid points, or species). There are some subtleties, like you have to create the correct boundary conditions before you load the initial guess. There is one method (loading an initial guess from an HDF file) that is currently not working, so this is anticipated. Because of issue Cantera#1288 in 2.6.0 the required version is set to 3.0

The ability to get an initial guess from an h5 file does not work. Removing the broken attempt, documentation, and example that suggests it should. As per ishoegl's suggestion Cantera#1293 (comment) For a work-around, restore a flame from the hdf5 file with flame.read_hdf then make a SolutionArray (or pandas.DataFrame) from that and use one of those to create an initial guess (both of which work)

- don't make it look like unit tests - print some output - keep under 88 characters - always create one solution, then save it in several formats

rwest · 2022-06-09T03:19:30Z

I think I have addressed all @ischoegl's comments and suggestions in the last 2 commits.

ischoegl · 2022-06-09T13:19:53Z

@rwest ... thanks for the updates (:+1: on the describe function!) ... I ended up looking into the HDF failure, which was much easier to fix than anticipated - there was a missing subgroup kwarg identifying the flame domain (a2f2fff). Also, the HDF cases require h5py, which is an optional dependency that needs to be handled gracefully (1f002c1). I added both commits to a branch on my own repo, see: https://github.com/ischoegl/cantera/tree/flamestart - feel free to cherry-pick!

Co-authored-by: Richard West <r.west@northeastern.edu>

rwest · 2022-06-09T13:44:12Z

Thanks. done.

ischoegl · 2022-06-09T14:29:16Z

Thanks! ... but - ugh - pandas is optional also. I pushed the fix here directly.

rwest · 2022-06-09T15:14:06Z

The pandas fix looks ok to me. Good catch. Thanks

ischoegl

@rwest - thanks for the fix and the example! It all looks good to me.

The ability to get an initial guess from an h5 file does not work. Removing the broken attempt, documentation, and example that suggests it should. As per ishoegl's suggestion #1293 (comment) For a work-around, restore a flame from the hdf5 file with flame.read_hdf then make a SolutionArray (or pandas.DataFrame) from that and use one of those to create an initial guess (both of which work)

ischoegl reviewed May 26, 2022

View reviewed changes

ischoegl added the Python label May 26, 2022

rwest force-pushed the flamestart branch from ad152c2 to abafa07 Compare May 31, 2022 13:12

ischoegl requested changes Jun 1, 2022

View reviewed changes

rwest added 4 commits June 7, 2022 09:19

Fix using pandas DataFrame for flame set_initial_guess

07dddef

Allows you to pass a pandas DataFrame as the data argument to a flame's set_initial_guess method. Closes Cantera#1288

[Python/examples] Suggestions from code review.

0a969e5

- don't make it look like unit tests - print some output - keep under 88 characters - always create one solution, then save it in several formats

rwest force-pushed the flamestart branch from abafa07 to 0a969e5 Compare June 9, 2022 03:16

[Python] Fix FlameBase.set_initial_guess from HDF

a2f2fff

[samples] Tweak flame_initial_guess example

3a6fd5f

Co-authored-by: Richard West <r.west@northeastern.edu>

[samples] Handle missing pandas dependency gracefully

ef32e50

ischoegl force-pushed the flamestart branch from baf75ad to ef32e50 Compare June 9, 2022 14:30

ischoegl approved these changes Jun 9, 2022

View reviewed changes

ischoegl merged commit 399142e into Cantera:main Jun 9, 2022

rwest deleted the flamestart branch June 9, 2022 15:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Flame restart and initial guess examples and fix issue #1288 #1293

Flame restart and initial guess examples and fix issue #1288 #1293

rwest commented May 26, 2022 •

edited

Loading

ischoegl left a comment

codecov bot commented May 31, 2022 •

edited

Loading

rwest commented May 31, 2022

ischoegl left a comment •

edited

Loading

rwest commented Jun 9, 2022

ischoegl commented Jun 9, 2022 •

edited

Loading

rwest commented Jun 9, 2022

ischoegl commented Jun 9, 2022

rwest commented Jun 9, 2022

ischoegl left a comment

Flame restart and initial guess examples and fix issue #1288 #1293

Flame restart and initial guess examples and fix issue #1288 #1293

Conversation

rwest commented May 26, 2022 • edited Loading

ischoegl left a comment

Choose a reason for hiding this comment

codecov bot commented May 31, 2022 • edited Loading

Codecov Report

rwest commented May 31, 2022

ischoegl left a comment • edited Loading

Choose a reason for hiding this comment

rwest commented Jun 9, 2022

ischoegl commented Jun 9, 2022 • edited Loading

rwest commented Jun 9, 2022

ischoegl commented Jun 9, 2022

rwest commented Jun 9, 2022

ischoegl left a comment

Choose a reason for hiding this comment

rwest commented May 26, 2022 •

edited

Loading

codecov bot commented May 31, 2022 •

edited

Loading

ischoegl left a comment •

edited

Loading

ischoegl commented Jun 9, 2022 •

edited

Loading