Skip to content

Fix 1D solver crash when loglevel > 7 (#2073) - #2162

Open
hazalkoom wants to merge 1 commit into
Cantera:mainfrom
hazalkoom:fix-1d-loglevel
Open

Fix 1D solver crash when loglevel > 7 (#2073)#2162
hazalkoom wants to merge 1 commit into
Cantera:mainfrom
hazalkoom:fix-1d-loglevel

Conversation

@hazalkoom

Copy link
Copy Markdown
Contributor

Changes proposed in this pull request

This pull request fixes a crash that happens when running the 1D flame solver with a debug log level greater than 7 (for example, loglevel=8).

  • The Problem: When loglevel is set to 8 or higher, the solver saves the mathematical errors (residuals) to debug_sim1d.yaml. To do this, saveResidual() temporarily copies error values into the main solution vector (m_state). However, when writing the output file, Flow1D::toArray() was calling updateThermo() on m_state. Because residual values are mathematical errors (not valid thermodynamic state data, and often contain zeros), this caused Cantera to throw temperature must be positive. T = 0 and crash.
  • The Fix: Added an optional skipThermo parameter (defaulting to false) to Domain1D::toArray() and its derived classes (Flow1D, Boundary1D). When saveResidual() outputs residual data, skipThermo is set to true, bypassing updateThermo() and density evaluations on non-physical residual numbers.
  • Unit Test: Added parameterized unit test test_solve_loglevel_residual to test/python/test_onedim.py testing debug output at loglevel 7 (solution) and 8 (residual) to prevent regressions.

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

Closes #2073

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

Not applicable (this is a bug fix).

AI Statement (required)

  • Extensive use of generative AI.
    Significant portions of code and documentation were generated using AI tools, and then reviewed and tested by the contributor. The planning phase was assisted by Claude Opus 4.6, and the execution and coding phase was completed using Gemini pro 3.1.

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
  • AI Statement is included
  • The pull request is ready for review

Copilot AI lite review requested due to automatic review settings August 7, 2026 09:02

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Fixes a crash in the 1D solver debug-output path when loglevel > 7 by allowing residual-saving to bypass thermodynamic state evaluation during toArray() serialization.

Changes:

  • Adds a skipThermo parameter to Domain1D::toArray() and propagates it through Flow1D / Boundary1D implementations.
  • Introduces an internal Sim1D::save(..., skipThermo) overload and uses it from saveResidual() to avoid evaluating thermo on residual vectors.
  • Adds a Python regression test intended to exercise loglevel 7 vs 8 debug output.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
test/python/test_onedim.py Adds a regression test for high loglevels and debug file behavior.
src/oneD/Sim1D.cpp Routes residual saving through toArray(..., skipThermo=true) via an internal save overload.
src/oneD/Flow1D.cpp Skips updateThermo() (and related density/normalization) when skipThermo=true.
src/oneD/Boundary1D.cpp Updates toArray signatures across boundary domains to accept skipThermo.
include/cantera/oneD/Sim1D.h Declares internal save(..., skipThermo) helper.
include/cantera/oneD/Flow1D.h Updates toArray override signature.
include/cantera/oneD/Domain1D.h Extends toArray API contract with skipThermo documentation.
include/cantera/oneD/Boundary1D.h Updates toArray override signatures for boundary domains.
Suppressed comments (1)

src/oneD/Boundary1D.cpp:767

  • The new skipThermo parameter is not used here. When Sim1D::saveResidual() swaps m_state to residuals, this toArray() will still call setCoverages() using non-physical residual values, which can throw and reintroduce the loglevel>7 crash for ReactingSurface1D cases. Handle skipThermo by avoiding any thermo/phase state updates and writing the raw values into the SolutionArray instead.
shared_ptr<SolutionArray> ReactingSurf1D::toArray(bool normalize, bool skipThermo)
{
    if (!m_state) {
        throw CanteraError("ReactingSurf1D::toArray",
            "Domain needs to be installed in a container before calling toArray.");

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread test/python/test_onedim.py Outdated
@hazalkoom

Copy link
Copy Markdown
Contributor Author

Addressed review comments and updated the PR:

  • Added skipThermo handling in ReactingSurf1D::toArray and updated SolutionArray::writeEntry to bypass thermodynamic state evaluation during residual vector serialization.
  • Refactored test_solve_loglevel_residual using pytest.monkeypatch to ensure temporary debug output files are written to test_work_path and cleaned up.
  • All 1D solver unit tests are passing cleanly.

@codecov

codecov Bot commented Aug 7, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 80.95238% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.19%. Comparing base (12526b1) to head (d138cd2).

Files with missing lines Patch % Lines
src/oneD/Boundary1D.cpp 76.00% 4 Missing and 2 partials ⚠️
src/base/SolutionArray.cpp 81.81% 3 Missing and 1 partial ⚠️
include/cantera/oneD/Domain1D.h 0.00% 1 Missing ⚠️
src/oneD/Flow1D.cpp 83.33% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2162      +/-   ##
==========================================
+ Coverage   78.15%   78.19%   +0.04%     
==========================================
  Files         453      453              
  Lines       55433    55461      +28     
  Branches     9115     9123       +8     
==========================================
+ Hits        43324    43370      +46     
+ Misses       9060     9041      -19     
- Partials     3049     3050       +1     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

* Added skipThermo parameter to Domain1D::toArray to avoid evaluating thermodynamic properties when saving solver residuals, which are not valid thermodynamic states.
* Updated Flow1D and Boundary1D classes override signatures.
* Added a parameterized unit test in test_onedim.py testing loglevel=7 and loglevel=8 to prevent regressions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1D solver crashes when loglevel >7

2 participants