From 534c2656dc00a80cea7140abdbe256e38421e820 Mon Sep 17 00:00:00 2001 From: Mohamed Ahmed Date: Mon, 10 Aug 2026 21:32:51 +0300 Subject: [PATCH] Fix 1D solver crash when loglevel > 7 (#2073) Fix crash caused by parallel file collision in test workers by using tmp_path, and AnyMap static caching. --- include/cantera/oneD/Domain1D.h | 6 ++++ include/cantera/oneD/Sim1D.h | 9 +++-- src/oneD/Domain1D.cpp | 19 +++++++++++ src/oneD/Sim1D.cpp | 32 +++++++++++------- test/oneD/test_oneD.cpp | 3 ++ test/python/test_onedim.py | 60 +++++++++++++++++++++++++++++++++ 6 files changed, 113 insertions(+), 16 deletions(-) diff --git a/include/cantera/oneD/Domain1D.h b/include/cantera/oneD/Domain1D.h index 0ec95c74f78..302a6c24a7f 100644 --- a/include/cantera/oneD/Domain1D.h +++ b/include/cantera/oneD/Domain1D.h @@ -526,6 +526,12 @@ class Domain1D throw NotImplementedError("Domain1D::toArray", "Needs to be overloaded."); } + //! Append residual data to a SolutionArray as extra components. + //! @param arr SolutionArray to append residual data to + //! @param res span of the local residual vector for this domain + //! @since New in %Cantera 4.0 + virtual void appendResiduals(SolutionArray& arr, span res) const; + //! Restore the solution for this domain from a SolutionArray. /*! * This method serves as an external interface for high-level API's. diff --git a/include/cantera/oneD/Sim1D.h b/include/cantera/oneD/Sim1D.h index b99c507cc1d..5260853376e 100644 --- a/include/cantera/oneD/Sim1D.h +++ b/include/cantera/oneD/Sim1D.h @@ -115,10 +115,13 @@ class Sim1D : public OneDim * ThermoPhase manager is used - @see nativeState (CSV only) */ void save(const string& fname, const string& name, const string& desc, - bool overwrite=false, int compression=0, const string& basis=""); + bool overwrite=false, int compression=0, const string& basis="", + const vector* res=nullptr); /** - * Save the residual of the current solution to a container file. + * Save the current solution and its residual vector to a container file. + * Residual values are appended to each domain's SolutionArray as extra components + * prefixed with `residual-`. * @param fname Name of output container file * @param name Identifier of solution within the container file * @param desc Description of the solution @@ -177,7 +180,7 @@ class Sim1D : public OneDim * file. * @param loglevel Controls the type of output that will be written. A `loglevel` * greater than 6 saves the solution, and a `loglevel` greater - * than 7 saves the residual additionally. + * than 7 saves the solution with residual vectors appended. * @param attempt_counter An integer counter used to uniquely identify the attempt * which is included in the file header to differentiate * between multiple solution attempts. diff --git a/src/oneD/Domain1D.cpp b/src/oneD/Domain1D.cpp index 16fb0b1c92f..b373ddba8dc 100644 --- a/src/oneD/Domain1D.cpp +++ b/src/oneD/Domain1D.cpp @@ -279,4 +279,23 @@ double Domain1D::initialValue(size_t n, size_t j) throw NotImplementedError("Domain1D::initialValue"); } +void Domain1D::appendResiduals(SolutionArray& arr, span res) const +{ + // Skip domains with no data points in the SolutionArray + if (arr.size() == 0) { + return; + } + for (size_t i = 0; i < nComponents(); i++) { + string rname = "residual-" + componentName(i); + arr.addExtra(rname); + vector data(nPoints()); + for (size_t j = 0; j < nPoints(); j++) { + data[j] = res[index(i, j)]; + } + AnyValue value; + value = data; + arr.setComponent(rname, value); + } +} + } // namespace diff --git a/src/oneD/Sim1D.cpp b/src/oneD/Sim1D.cpp index 80402b2926c..ab92cde6f94 100644 --- a/src/oneD/Sim1D.cpp +++ b/src/oneD/Sim1D.cpp @@ -60,7 +60,8 @@ double Sim1D::_workValue(size_t dom, size_t comp, size_t localPoint) const } void Sim1D::save(const string& fname, const string& name, const string& desc, - bool overwrite, int compression, const string& basis) + bool overwrite, int compression, const string& basis, + const vector* res) { size_t dot = fname.find_last_of("."); string extension = (dot != npos) ? toLowerCopy(fname.substr(dot+1)) : ""; @@ -82,6 +83,10 @@ void Sim1D::save(const string& fname, const string& name, const string& desc, SolutionArray::writeHeader(fname, name, desc, overwrite); for (auto dom : m_dom) { auto arr = dom->toArray(); + if (res) { + dom->appendResiduals(*arr, span( + res->data() + dom->loc(), dom->size())); + } arr->writeEntry(fname, name, dom->id(), overwrite, compression); } return; @@ -96,6 +101,10 @@ void Sim1D::save(const string& fname, const string& name, const string& desc, for (auto dom : m_dom) { auto arr = dom->toArray(); + if (res) { + dom->appendResiduals(*arr, span( + res->data() + dom->loc(), dom->size())); + } arr->writeEntry(data, name, dom->id(), overwrite); } @@ -113,12 +122,7 @@ void Sim1D::saveResidual(const string& fname, const string& name, { vector res(m_state->size(), -999); OneDim::eval(npos, *m_state, res, 0.0); - // Temporarily put the residual into m_state, since this is the vector that the - // save() function reads. - vector backup(*m_state); - *m_state = res; - save(fname, name, desc, overwrite, compression); - *m_state = backup; + save(fname, name, desc, overwrite, compression, "", &res); } namespace { // restrict scope of helper function to local translation unit @@ -503,20 +507,22 @@ int Sim1D::refine(int loglevel) void Sim1D::clearDebugFile() { + try { + AnyMap::clearCachedFile("debug_sim1d.yaml"); + } catch (CanteraError&) { + // File might not exist yet + } std::filesystem::remove("debug_sim1d.yaml"); } void Sim1D::writeDebugInfo(const string& header_suffix, const string& message, int loglevel, int attempt_counter) { - string file_header; - if (loglevel > 6) { - file_header = fmt::format("solution_{}_{}", attempt_counter, header_suffix); - save("debug_sim1d.yaml", file_header, message, true); - } + string file_header = fmt::format("solution_{}_{}", attempt_counter, header_suffix); if (loglevel > 7) { - file_header = fmt::format("residual_{}_{}", attempt_counter, header_suffix); saveResidual("debug_sim1d.yaml", file_header, message, true); + } else if (loglevel > 6) { + save("debug_sim1d.yaml", file_header, message, true); } } diff --git a/test/oneD/test_oneD.cpp b/test/oneD/test_oneD.cpp index 29318a4bcf3..1e9d1570cdb 100644 --- a/test/oneD/test_oneD.cpp +++ b/test/oneD/test_oneD.cpp @@ -82,9 +82,12 @@ TEST(onedim, freeflame) int loglevel = 0; flame->solve(loglevel, refine_grid); flame->save("gtest-freeflame.yaml", "cpp", "Solution from C++ interface", true); + flame->saveResidual("gtest-freeflame-res.yaml", "cpp_res", "Residual from C++ interface", true); if (usesHDF5()) { flame->save("gtest-freeflame.h5", "cpp", "Solution from C++ interface", true); + flame->saveResidual("gtest-freeflame-res.h5", "cpp_res", "Residual from C++ interface", true); } + EXPECT_THROW(flame->saveResidual("gtest-freeflame-res.bad", "cpp_res", "Residual", true), CanteraError); ASSERT_EQ(flow->nPoints(), static_cast(nz + 1)); auto Tvec = flow->values("T"); diff --git a/test/python/test_onedim.py b/test/python/test_onedim.py index 9c97fefe912..afc9c8fd0e3 100644 --- a/test/python/test_onedim.py +++ b/test/python/test_onedim.py @@ -1004,6 +1004,26 @@ def test_save_restore_hdf(self): def test_save_restore_yaml(self): self.run_save_restore("yaml") + @pytest.mark.parametrize("loglevel", [7, 8]) + def test_solve_loglevel_residual(self, tmp_path, monkeypatch, loglevel, capsys): + # Test high loglevels (7=solution, 8=residual) to prevent regressions (#2073) + monkeypatch.chdir(tmp_path) + self.run_mix(phi=1.0, T=300, width=0.05, p=1.0, refine=False) + self.sim.solve(loglevel=loglevel, refine_grid=False) + capsys.readouterr() # discard log output + debug_file = tmp_path / "debug_sim1d.yaml" + assert debug_file.exists() + + debug = ct.SolutionArray(self.gas) + debug.restore(debug_file, "solution_1_NewtonSuccess/flame") + assert debug.shape == (self.sim.flame.n_points,) + + if loglevel > 7: + assert "residual-T" in debug.component_names + assert debug.residual_T.shape == (self.sim.flame.n_points,) + else: + assert "residual-T" not in debug.component_names + def run_save_restore(self, mode): filename = self.test_work_path / f"freeflame.{mode}" filename.unlink(missing_ok=True) @@ -2136,6 +2156,46 @@ def run_reacting_surface(self, xch4, tsurf, mdot, width): def test_reacting_surface_case1(self): self.run_reacting_surface(xch4=0.095, tsurf=900.0, mdot=0.06, width=0.1) + @pytest.mark.parametrize("loglevel", [7, 8]) + def test_reacting_surface_loglevel_residual(self, tmp_path, monkeypatch, loglevel, + capsys): + monkeypatch.chdir(tmp_path) + comp = {'CH4': 0.095, 'O2': 0.21, 'N2': 0.79} + sim = self.create_reacting_surface(comp, tsurf=900.0, tinlet=300.0, width=0.1) + sim.inlet.mdot = 0.06 + sim.inlet.T = 300.0 + sim.inlet.X = comp + sim.surface.T = 900.0 + sim.solve(loglevel=loglevel, refine_grid=False) + capsys.readouterr() # discard log output + debug_file = tmp_path / "debug_sim1d.yaml" + assert debug_file.exists() + + with open(debug_file, "r") as f: + debug_text = f.read() + + success_keys = [] + for line in debug_text.splitlines(): + if line.startswith("solution_") and line.endswith(":"): + key = line.split(":")[0] + if "NewtonSuccess" in key: + success_keys.append(key) + + node_key = success_keys[-1] if success_keys else "solution_1_NewtonSuccess" + + debug_flame = ct.SolutionArray(self.gas) + debug_flame.restore(debug_file, f"{node_key}/flame") + assert debug_flame.shape == (sim.flame.n_points,) + + debug_surf = ct.SolutionArray(sim.surface.phase) + debug_surf.restore(debug_file, f"{node_key}/surface") + assert debug_surf.shape == (sim.surface.n_points,) + + if loglevel > 7: + assert "residual-T" in debug_flame.component_names + else: + assert "residual-T" not in debug_flame.component_names + @pytest.mark.slow_test def test_reacting_surface_case2(self): self.run_reacting_surface(xch4=0.07, tsurf=1200.0, mdot=0.2, width=0.05)