From ed2accf418b72b467ef102a47c930689339c236d Mon Sep 17 00:00:00 2001 From: Anthony Walker Date: Wed, 16 Jun 2021 15:43:33 -0400 Subject: [PATCH 1/8] Fixing segfault caused by initialization of single reactor There was a segmentation fault when calling initialize on an individual reactor, this is a fix for that by setting it to the specified time if the network is not available --- include/cantera/zeroD/Reactor.h | 3 ++- src/zeroD/Reactor.cpp | 6 +++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/include/cantera/zeroD/Reactor.h b/include/cantera/zeroD/Reactor.h index 8f4b08a640..4a4d2d04e7 100644 --- a/include/cantera/zeroD/Reactor.h +++ b/include/cantera/zeroD/Reactor.h @@ -193,7 +193,8 @@ class Reactor : public ReactorBase //! `true` for reactors where the pressure is a dependent property, //! calculated from the state, and `false` when the pressure is constant //! or an independent variable. - virtual void updateConnected(bool updatePressure); + //! @param t0 initialization time for the reactor if not obtained from the network + virtual void updateConnected(bool updatePressure, double t0 = 0.0); //! Get initial conditions for SurfPhase objects attached to this reactor virtual void getSurfaceInitialConditions(double* y); diff --git a/src/zeroD/Reactor.cpp b/src/zeroD/Reactor.cpp index dd0080dd2e..ce917ccb36 100644 --- a/src/zeroD/Reactor.cpp +++ b/src/zeroD/Reactor.cpp @@ -88,7 +88,7 @@ void Reactor::initialize(doublereal t0) m_thermo->restoreState(m_state); m_sdot.resize(m_nsp, 0.0); m_wdot.resize(m_nsp, 0.0); - updateConnected(true); + updateConnected(true,t0); for (size_t n = 0; n < m_wall.size(); n++) { WallBase* W = m_wall[n]; @@ -177,7 +177,7 @@ void Reactor::updateSurfaceState(double* y) } } -void Reactor::updateConnected(bool updatePressure) { +void Reactor::updateConnected(bool updatePressure, double t0) { // save parameters needed by other connected reactors m_enthalpy = m_thermo->enthalpy_mass(); if (updatePressure) { @@ -187,7 +187,7 @@ void Reactor::updateConnected(bool updatePressure) { m_thermo->saveState(m_state); // Update the mass flow rate of connected flow devices - double time = m_net->time(); + double time = (m_net!=NULL) ? m_net->time() : t0; for (size_t i = 0; i < m_outlet.size(); i++) { m_outlet[i]->updateMassFlowRate(time); } From 5b97f8064c67ffaa5b71fe105f2d78b77048139c Mon Sep 17 00:00:00 2001 From: Anthony Walker Date: Thu, 17 Jun 2021 11:17:05 -0400 Subject: [PATCH 2/8] Adding a test for single reactor initialize fix. I adapted the example ./interfaces/cython/cantera/test_reactor.py::test_equilibrium_HP to C++ and performed the reactor initialization with an arbitrary value to show that it no longer creates a segmentation fault and also still integrates successfully in a network post individual initialization. I added this to it's own folder called zeroD because I could not find C++ instances of Reactor tests elsewhere. --- include/cantera/zeroD/Reactor.h | 2 +- src/zeroD/Reactor.cpp | 4 +-- test/SConscript | 1 + test/zeroD/test_zeroD.cpp | 57 +++++++++++++++++++++++++++++++++ 4 files changed, 61 insertions(+), 3 deletions(-) create mode 100644 test/zeroD/test_zeroD.cpp diff --git a/include/cantera/zeroD/Reactor.h b/include/cantera/zeroD/Reactor.h index 4a4d2d04e7..3177809642 100644 --- a/include/cantera/zeroD/Reactor.h +++ b/include/cantera/zeroD/Reactor.h @@ -194,7 +194,7 @@ class Reactor : public ReactorBase //! calculated from the state, and `false` when the pressure is constant //! or an independent variable. //! @param t0 initialization time for the reactor if not obtained from the network - virtual void updateConnected(bool updatePressure, double t0 = 0.0); + virtual void updateConnected(bool updatePressure, double t0=0.0); //! Get initial conditions for SurfPhase objects attached to this reactor virtual void getSurfaceInitialConditions(double* y); diff --git a/src/zeroD/Reactor.cpp b/src/zeroD/Reactor.cpp index ce917ccb36..e60f1dba36 100644 --- a/src/zeroD/Reactor.cpp +++ b/src/zeroD/Reactor.cpp @@ -88,7 +88,7 @@ void Reactor::initialize(doublereal t0) m_thermo->restoreState(m_state); m_sdot.resize(m_nsp, 0.0); m_wdot.resize(m_nsp, 0.0); - updateConnected(true,t0); + updateConnected(true, t0); for (size_t n = 0; n < m_wall.size(); n++) { WallBase* W = m_wall[n]; @@ -187,7 +187,7 @@ void Reactor::updateConnected(bool updatePressure, double t0) { m_thermo->saveState(m_state); // Update the mass flow rate of connected flow devices - double time = (m_net!=NULL) ? m_net->time() : t0; + double time = (m_net != nullptr) ? m_net->time() : t0; for (size_t i = 0; i < m_outlet.size(); i++) { m_outlet[i]->updateMassFlowRate(time); } diff --git a/test/SConscript b/test/SConscript index a4979b6223..1ecb5df2d6 100644 --- a/test/SConscript +++ b/test/SConscript @@ -290,6 +290,7 @@ addTestProgram('thermo', 'thermo') addTestProgram('equil', 'equil') addTestProgram('kinetics', 'kinetics') addTestProgram('transport', 'transport') +addTestProgram('zeroD', 'zeroD') python_subtests = [''] test_root = '#interfaces/cython/cantera/test' diff --git a/test/zeroD/test_zeroD.cpp b/test/zeroD/test_zeroD.cpp new file mode 100644 index 0000000000..154c8a5fe7 --- /dev/null +++ b/test/zeroD/test_zeroD.cpp @@ -0,0 +1,57 @@ +#include +#include +#include "gtest/gtest.h" +#include "cantera/thermo.h" +#include "cantera/kinetics.h" +#include "cantera/zerodim.h" + +using namespace Cantera; + +//This test ensures that prior reactor initialization of a reactor does not affect later integration within a network. This example was adapted from test_reactor.py::test_equilibrium_HP. +TEST(ZeroDim, test_individual_reactor_initialization) +{ + //Initial conditions + double T0 = 1100.0; + double P0 = 1013250; + std::string X0 = "H2:1.0, O2:0.5, AR:8.0"; + //IdealGasConstPressureReactor solution, phase, and kinetics objects + std::shared_ptr sol1 = newSolution("h2o2.yaml"); + std::shared_ptr gas1 = sol1->thermo(); + std::shared_ptr kin1 = sol1->kinetics(); + gas1->setState_TPX(T0, P0, X0); + //Set up reactor object + IdealGasConstPressureReactor reactor1; + reactor1.setKineticsMgr(*kin1); + reactor1.setThermoMgr(*gas1); + //initialize reactor at arbitrary value + reactor1.initialize(0.1); + //Setup reactor network and integrating + ReactorNet network; + network.addReactor(reactor1); + network.advance(1.0); + //Secondary gas for comparison + std::shared_ptr sol2 = newSolution("h2o2.yaml"); + std::shared_ptr gas2 = sol2->thermo(); + std::shared_ptr kin2 = sol2->kinetics(); + gas2->setState_TPX(T0, P0, X0); + gas2->equilibrate("HP"); + //Compare the two gases. + double tol = 1e-8; //relative tolerance from python assertNear test + EXPECT_NEAR(gas1->temperature(), gas2->temperature(), tol); + EXPECT_NEAR(gas1->density(), gas2->density(), tol); + EXPECT_NEAR(gas1->pressure(), P0, tol); + for(size_t i = 0; i < gas1->stateSize()-2; i++) + { + EXPECT_NEAR(gas1->massFraction(i), gas2->massFraction(i), tol); + } +} + +int main(int argc, char** argv) +{ + printf("Running main() from test_zeroD.cpp\n"); + testing::InitGoogleTest(&argc, argv); + Cantera::make_deprecation_warnings_fatal(); + int result = RUN_ALL_TESTS(); + Cantera::appdelete(); + return result; +} From 2f4681a4954e83196f6d6b7a4f5430cbe122a550 Mon Sep 17 00:00:00 2001 From: Anthony Walker Date: Fri, 18 Jun 2021 11:50:25 -0400 Subject: [PATCH 3/8] This commit makes requested changes to the reactor initialization test. This commit changes the former test to use Reactor classes in lieu of IdealGasConstPressureReactor classes because it is the base class. It also changes some of the comparisons between the two reactor objects. --- test/zeroD/test_zeroD.cpp | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/test/zeroD/test_zeroD.cpp b/test/zeroD/test_zeroD.cpp index 154c8a5fe7..71116e64c9 100644 --- a/test/zeroD/test_zeroD.cpp +++ b/test/zeroD/test_zeroD.cpp @@ -14,13 +14,13 @@ TEST(ZeroDim, test_individual_reactor_initialization) double T0 = 1100.0; double P0 = 1013250; std::string X0 = "H2:1.0, O2:0.5, AR:8.0"; - //IdealGasConstPressureReactor solution, phase, and kinetics objects + //Reactor solution, phase, and kinetics objects std::shared_ptr sol1 = newSolution("h2o2.yaml"); std::shared_ptr gas1 = sol1->thermo(); std::shared_ptr kin1 = sol1->kinetics(); gas1->setState_TPX(T0, P0, X0); //Set up reactor object - IdealGasConstPressureReactor reactor1; + Reactor reactor1; reactor1.setKineticsMgr(*kin1); reactor1.setThermoMgr(*gas1); //initialize reactor at arbitrary value @@ -34,15 +34,25 @@ TEST(ZeroDim, test_individual_reactor_initialization) std::shared_ptr gas2 = sol2->thermo(); std::shared_ptr kin2 = sol2->kinetics(); gas2->setState_TPX(T0, P0, X0); - gas2->equilibrate("HP"); - //Compare the two gases. - double tol = 1e-8; //relative tolerance from python assertNear test - EXPECT_NEAR(gas1->temperature(), gas2->temperature(), tol); - EXPECT_NEAR(gas1->density(), gas2->density(), tol); - EXPECT_NEAR(gas1->pressure(), P0, tol); - for(size_t i = 0; i < gas1->stateSize()-2; i++) + gas2->equilibrate("UV"); + //Secondary reactor for comparison + Reactor reactor2; + reactor2.setKineticsMgr(*kin2); + reactor2.setThermoMgr(*gas2); + reactor2.initialize(0); + //Get state of reactors + double state1 [reactor1.neq()]; + double state2 [reactor2.neq()]; + reactor1.getState(state1); + reactor2.getState(state2); + //Compare the reactors. + EXPECT_EQ(reactor1.neq(), reactor2.neq()); + double tol = 1e-14; + EXPECT_NEAR(state1[0], state2[0], tol); + EXPECT_NEAR(state1[1], state2[1], tol); + for(size_t i = 3; i < reactor2.neq(); i++) { - EXPECT_NEAR(gas1->massFraction(i), gas2->massFraction(i), tol); + EXPECT_NEAR(state1[i], state2[i], tol); } } From e24d9777bbf0fa861683e0ca746f9afa4c2d3710 Mon Sep 17 00:00:00 2001 From: Anthony Walker Date: Sat, 19 Jun 2021 18:46:40 -0400 Subject: [PATCH 4/8] Makes requested formatting changes and fixes CI. Specifically, it ends lines of at 88 characters and adds space after inline comments. It also changed the allocation of arrays to satisfy CI. --- test/zeroD/test_zeroD.cpp | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/test/zeroD/test_zeroD.cpp b/test/zeroD/test_zeroD.cpp index 71116e64c9..af0089bc56 100644 --- a/test/zeroD/test_zeroD.cpp +++ b/test/zeroD/test_zeroD.cpp @@ -7,53 +7,57 @@ using namespace Cantera; -//This test ensures that prior reactor initialization of a reactor does not affect later integration within a network. This example was adapted from test_reactor.py::test_equilibrium_HP. +// This test ensures that prior reactor initialization of a reactor does +// not affect later integration within a network. This example was +// adapted from test_reactor.py::test_equilibrium_HP. TEST(ZeroDim, test_individual_reactor_initialization) { - //Initial conditions + // Initial conditions double T0 = 1100.0; double P0 = 1013250; std::string X0 = "H2:1.0, O2:0.5, AR:8.0"; - //Reactor solution, phase, and kinetics objects + // Reactor solution, phase, and kinetics objects std::shared_ptr sol1 = newSolution("h2o2.yaml"); std::shared_ptr gas1 = sol1->thermo(); std::shared_ptr kin1 = sol1->kinetics(); gas1->setState_TPX(T0, P0, X0); - //Set up reactor object + // Set up reactor object Reactor reactor1; reactor1.setKineticsMgr(*kin1); reactor1.setThermoMgr(*gas1); - //initialize reactor at arbitrary value + // Initialize reactor at arbitrary value prior to integration to ensure no impact reactor1.initialize(0.1); - //Setup reactor network and integrating + // Setup reactor network and integrate ReactorNet network; network.addReactor(reactor1); network.advance(1.0); - //Secondary gas for comparison + // Secondary gas for comparison std::shared_ptr sol2 = newSolution("h2o2.yaml"); std::shared_ptr gas2 = sol2->thermo(); std::shared_ptr kin2 = sol2->kinetics(); gas2->setState_TPX(T0, P0, X0); gas2->equilibrate("UV"); - //Secondary reactor for comparison + // Secondary reactor for comparison Reactor reactor2; reactor2.setKineticsMgr(*kin2); reactor2.setThermoMgr(*gas2); reactor2.initialize(0); - //Get state of reactors - double state1 [reactor1.neq()]; - double state2 [reactor2.neq()]; + // Get state of reactors + double *state1 = new double[reactor1.neq()]; + double *state2 = new double[reactor2.neq()]; reactor1.getState(state1); reactor2.getState(state2); - //Compare the reactors. + // Compare the reactors. EXPECT_EQ(reactor1.neq(), reactor2.neq()); double tol = 1e-14; EXPECT_NEAR(state1[0], state2[0], tol); EXPECT_NEAR(state1[1], state2[1], tol); - for(size_t i = 3; i < reactor2.neq(); i++) + for(size_t i = 3; i < reactor1.neq(); i++) { EXPECT_NEAR(state1[i], state2[i], tol); } + delete[] state1; + delete[] state2; } int main(int argc, char** argv) From 681346556c3c62f281147ba5e407098f68669339 Mon Sep 17 00:00:00 2001 From: Anthony Walker Date: Sat, 19 Jun 2021 23:29:16 -0400 Subject: [PATCH 5/8] Using vector as a fix for CI instead of raw double pointer --- test/zeroD/test_zeroD.cpp | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/test/zeroD/test_zeroD.cpp b/test/zeroD/test_zeroD.cpp index af0089bc56..33f0bcccc1 100644 --- a/test/zeroD/test_zeroD.cpp +++ b/test/zeroD/test_zeroD.cpp @@ -43,10 +43,10 @@ TEST(ZeroDim, test_individual_reactor_initialization) reactor2.setThermoMgr(*gas2); reactor2.initialize(0); // Get state of reactors - double *state1 = new double[reactor1.neq()]; - double *state2 = new double[reactor2.neq()]; - reactor1.getState(state1); - reactor2.getState(state2); + std::vector state1 (reactor1.neq()); + std::vector state2 (reactor2.neq()); + reactor1.getState(state1.data()); + reactor2.getState(state2.data()); // Compare the reactors. EXPECT_EQ(reactor1.neq(), reactor2.neq()); double tol = 1e-14; @@ -56,8 +56,6 @@ TEST(ZeroDim, test_individual_reactor_initialization) { EXPECT_NEAR(state1[i], state2[i], tol); } - delete[] state1; - delete[] state2; } int main(int argc, char** argv) From 91602e9854661553f3303147e89988ef9ec48408 Mon Sep 17 00:00:00 2001 From: Anthony Walker Date: Mon, 21 Jun 2021 10:36:05 -0400 Subject: [PATCH 6/8] Remove t0 from updateConnected function call --- include/cantera/zeroD/Reactor.h | 3 +-- src/zeroD/Reactor.cpp | 6 +++--- test/zeroD/test_zeroD.cpp | 6 +++--- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/include/cantera/zeroD/Reactor.h b/include/cantera/zeroD/Reactor.h index 3177809642..8f4b08a640 100644 --- a/include/cantera/zeroD/Reactor.h +++ b/include/cantera/zeroD/Reactor.h @@ -193,8 +193,7 @@ class Reactor : public ReactorBase //! `true` for reactors where the pressure is a dependent property, //! calculated from the state, and `false` when the pressure is constant //! or an independent variable. - //! @param t0 initialization time for the reactor if not obtained from the network - virtual void updateConnected(bool updatePressure, double t0=0.0); + virtual void updateConnected(bool updatePressure); //! Get initial conditions for SurfPhase objects attached to this reactor virtual void getSurfaceInitialConditions(double* y); diff --git a/src/zeroD/Reactor.cpp b/src/zeroD/Reactor.cpp index e60f1dba36..8a6785fd2a 100644 --- a/src/zeroD/Reactor.cpp +++ b/src/zeroD/Reactor.cpp @@ -88,7 +88,7 @@ void Reactor::initialize(doublereal t0) m_thermo->restoreState(m_state); m_sdot.resize(m_nsp, 0.0); m_wdot.resize(m_nsp, 0.0); - updateConnected(true, t0); + updateConnected(true); for (size_t n = 0; n < m_wall.size(); n++) { WallBase* W = m_wall[n]; @@ -177,7 +177,7 @@ void Reactor::updateSurfaceState(double* y) } } -void Reactor::updateConnected(bool updatePressure, double t0) { +void Reactor::updateConnected(bool updatePressure) { // save parameters needed by other connected reactors m_enthalpy = m_thermo->enthalpy_mass(); if (updatePressure) { @@ -187,7 +187,7 @@ void Reactor::updateConnected(bool updatePressure, double t0) { m_thermo->saveState(m_state); // Update the mass flow rate of connected flow devices - double time = (m_net != nullptr) ? m_net->time() : t0; + double time = (m_net != nullptr) ? m_net->time() : 0.0; for (size_t i = 0; i < m_outlet.size(); i++) { m_outlet[i]->updateMassFlowRate(time); } diff --git a/test/zeroD/test_zeroD.cpp b/test/zeroD/test_zeroD.cpp index 33f0bcccc1..f045547608 100644 --- a/test/zeroD/test_zeroD.cpp +++ b/test/zeroD/test_zeroD.cpp @@ -25,8 +25,8 @@ TEST(ZeroDim, test_individual_reactor_initialization) Reactor reactor1; reactor1.setKineticsMgr(*kin1); reactor1.setThermoMgr(*gas1); - // Initialize reactor at arbitrary value prior to integration to ensure no impact - reactor1.initialize(0.1); + // Initialize reactor prior to integration to ensure no impact + reactor1.initialize(); // Setup reactor network and integrate ReactorNet network; network.addReactor(reactor1); @@ -41,7 +41,7 @@ TEST(ZeroDim, test_individual_reactor_initialization) Reactor reactor2; reactor2.setKineticsMgr(*kin2); reactor2.setThermoMgr(*gas2); - reactor2.initialize(0); + reactor2.initialize(0.0); // Get state of reactors std::vector state1 (reactor1.neq()); std::vector state2 (reactor2.neq()); From 4870ead0ec5bec3473b09b0293b9d69a53c0e834 Mon Sep 17 00:00:00 2001 From: Anthony Walker Date: Fri, 25 Jun 2021 14:42:39 -0400 Subject: [PATCH 7/8] This commit applies a suggested simplification to the test. It specifically changes the application of the phase and kinetics objects to the reactor by using "insert" instead of setKineticMgr and setThermoMgr. It also changes the initial pressure to use a built in constant in cantera. --- test/zeroD/test_zeroD.cpp | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/test/zeroD/test_zeroD.cpp b/test/zeroD/test_zeroD.cpp index f045547608..1045a5c7af 100644 --- a/test/zeroD/test_zeroD.cpp +++ b/test/zeroD/test_zeroD.cpp @@ -14,17 +14,14 @@ TEST(ZeroDim, test_individual_reactor_initialization) { // Initial conditions double T0 = 1100.0; - double P0 = 1013250; + double P0 = 10*OneAtm; std::string X0 = "H2:1.0, O2:0.5, AR:8.0"; // Reactor solution, phase, and kinetics objects std::shared_ptr sol1 = newSolution("h2o2.yaml"); - std::shared_ptr gas1 = sol1->thermo(); - std::shared_ptr kin1 = sol1->kinetics(); - gas1->setState_TPX(T0, P0, X0); + sol1->thermo()->setState_TPX(T0, P0, X0); // Set up reactor object Reactor reactor1; - reactor1.setKineticsMgr(*kin1); - reactor1.setThermoMgr(*gas1); + reactor1.insert(sol1); // Initialize reactor prior to integration to ensure no impact reactor1.initialize(); // Setup reactor network and integrate @@ -33,14 +30,11 @@ TEST(ZeroDim, test_individual_reactor_initialization) network.advance(1.0); // Secondary gas for comparison std::shared_ptr sol2 = newSolution("h2o2.yaml"); - std::shared_ptr gas2 = sol2->thermo(); - std::shared_ptr kin2 = sol2->kinetics(); - gas2->setState_TPX(T0, P0, X0); - gas2->equilibrate("UV"); + sol2->thermo()->setState_TPX(T0, P0, X0); + sol2->thermo()->equilibrate("UV"); // Secondary reactor for comparison Reactor reactor2; - reactor2.setKineticsMgr(*kin2); - reactor2.setThermoMgr(*gas2); + reactor2.insert(sol2); reactor2.initialize(0.0); // Get state of reactors std::vector state1 (reactor1.neq()); From 9fec84f51b62a132976910a7b2f7d3bfa661db6e Mon Sep 17 00:00:00 2001 From: Anthony Walker Date: Fri, 25 Jun 2021 21:23:17 -0400 Subject: [PATCH 8/8] Adjusted the test tolerance and completed suggested formatting changes --- test/zeroD/test_zeroD.cpp | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/test/zeroD/test_zeroD.cpp b/test/zeroD/test_zeroD.cpp index 1045a5c7af..c9b97631f4 100644 --- a/test/zeroD/test_zeroD.cpp +++ b/test/zeroD/test_zeroD.cpp @@ -14,7 +14,8 @@ TEST(ZeroDim, test_individual_reactor_initialization) { // Initial conditions double T0 = 1100.0; - double P0 = 10*OneAtm; + double P0 = 10 * OneAtm; + double tol = 1e-7; std::string X0 = "H2:1.0, O2:0.5, AR:8.0"; // Reactor solution, phase, and kinetics objects std::shared_ptr sol1 = newSolution("h2o2.yaml"); @@ -43,10 +44,7 @@ TEST(ZeroDim, test_individual_reactor_initialization) reactor2.getState(state2.data()); // Compare the reactors. EXPECT_EQ(reactor1.neq(), reactor2.neq()); - double tol = 1e-14; - EXPECT_NEAR(state1[0], state2[0], tol); - EXPECT_NEAR(state1[1], state2[1], tol); - for(size_t i = 3; i < reactor1.neq(); i++) + for (size_t i = 0; i < reactor1.neq(); i++) { EXPECT_NEAR(state1[i], state2[i], tol); }