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

Counterflow solution strategy #1463

Merged
merged 1 commit into from
Apr 5, 2023

Conversation

g3bk47
Copy link
Contributor

@g3bk47 g3bk47 commented Mar 22, 2023

Currently, an "extinct flame" solution is treated as successful solution for counterflow flames. Therefore, trying to find a solution by first disabling the energy equation is never tried. However, this can significantly increase the chance of finding the non-extinct solution. This PR modifies the solution strategy for counterflow flames so that after an extinct flame solution is found, the normal workflow of trying another solution with disabled energy equation is applied. Previous discussion can be found in Cantera/enhancements#155.

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 22, 2023

Codecov Report

Merging #1463 (aac4bf1) into main (8b5a0e9) will increase coverage by 0.03%.
The diff coverage is 100.00%.

❗ Current head aac4bf1 differs from pull request most recent head a570f71. Consider uploading reports for the commit a570f71 to get more accurate results

@@            Coverage Diff             @@
##             main    #1463      +/-   ##
==========================================
+ Coverage   69.86%   69.90%   +0.03%     
==========================================
  Files         377      377              
  Lines       57298    57301       +3     
  Branches    19164    19164              
==========================================
+ Hits        40033    40054      +21     
+ Misses      14712    14694      -18     
  Partials     2553     2553              
Impacted Files Coverage Δ
interfaces/cython/cantera/_onedim.pyx 81.05% <100.00%> (+1.70%) ⬆️

... and 1 file with indirect coverage changes

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

Copy link
Member

@ischoegl ischoegl left a comment

Choose a reason for hiding this comment

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

@g3bk47 … thank you for the PR! Apart from squashing the ‘fixing line length’ commit, could you comment on the odd dip in temperature for U=7 m/s in your original post Cantera/enhancements#155 (comment)? I’m a little concerned as is looks like an artifact that we’d like to avoid. I realize that you pointed this out yourself …

@g3bk47 g3bk47 force-pushed the CounterflowSolutionStrategy branch from aac4bf1 to a570f71 Compare April 4, 2023 17:14
@g3bk47
Copy link
Contributor Author

g3bk47 commented Apr 4, 2023

Hi @ischoegl,
I merged the two commits and took a closer look at the example from Cantera/enhancements#155:
The cause for the odd dip (black dashed line below) was that I set the tolerances to very high values in my test script so that some of the flame "solutions" contained only a hand full of points. After changing
f.set_refine_criteria(ratio=4, slope=0.2, curve=0.3, prune=0.04)
to
f.set_refine_criteria(ratio=2.5, slope=0.05, curve=0.05, prune=0.005)
I get pretty much perfect agreement between the direct solution using the new code (blue solid line) and the continuation method (red dotted line) in terms of maximum temperature:
T

Copy link
Member

@ischoegl ischoegl left a comment

Choose a reason for hiding this comment

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

Thanks, @g3bk47 ... this looks good to me, and the explanation makes sense. Regarding the CI failures, they are unrelated - see #1471.

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.

2 participants