Skip to content

Commit

Permalink
Fixed issue where additional args could deallocate while CySolver sti…
Browse files Browse the repository at this point in the history
…ll pointed to them.
  • Loading branch information
jrenaud90 committed Nov 28, 2024
1 parent 21724da commit 4b1abd7
Show file tree
Hide file tree
Showing 21 changed files with 1,178 additions and 251 deletions.
15 changes: 14 additions & 1 deletion CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,24 @@

## 2024

#### v0.11.6 (2024-NNN)
#### v0.12.0 (2024-NNN)

New:
* `MAX_STEP` can now be cimported from the top-level of CyRK: "from CyRK cimport MAX_STEP"

Fixes:
* Fixed issue where derived solvers (RK solvers) were not calling `CySolverBase` parent class' destructor.
* Fixed issue with `cysolve_ivp` (`pysolve_ivp` did not have this bug) where additional args are passed to diffeq _and_ dense output is on _and_ extra output is captured.
* Calling the dense output when extra output is on requires additional calls to the diffeq. However, after integration there is no gurantee that the args pointer is pointing to the same memory or that the values in that memory have not changed.
* Best case, this could cause unexpected results as new values are used for additional args; worst case it could cause access violations if the diffeq tries to access released memory.
* Now the `CySolverBase` makes a copy of the additional argument structure so it always retains those values as long as it is alive. This requires an additional memory allocation at the start of integration. And it requires the user to provide the size of the additional argument structure to `cysolve_ivp`.

Tests:
* Fixed tests where additional args were not being used.

Documentation:
* Updated the "Advanced CySolver.md" documentation that was out of date.

#### v0.11.5 (2024-11-27)

New:
Expand Down
2 changes: 1 addition & 1 deletion CyRK/__init__.pxd
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
from CyRK.cy.cysolver_api cimport cysolve_ivp, cysolve_ivp_gil, cysolve_ivp_noreturn, DiffeqFuncType, PreEvalFunc, CySolverResult, WrapCySolverResult, CySolverBase, CySolveOutput, RK23_METHOD_INT, RK45_METHOD_INT, DOP853_METHOD_INT
from CyRK.cy.cysolver_api cimport cysolve_ivp, cysolve_ivp_gil, cysolve_ivp_noreturn, DiffeqFuncType, PreEvalFunc, CySolverResult, WrapCySolverResult, CySolverBase, CySolveOutput, RK23_METHOD_INT, RK45_METHOD_INT, DOP853_METHOD_INT, MAX_STEP
from CyRK.cy.helpers cimport interpolate_from_solution_list
11 changes: 6 additions & 5 deletions CyRK/cy/cysolution.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ void CySolverResult::build_solver(
// General optional arguments
const size_t expected_size,
const void* args_ptr,
const size_t size_of_args,
const size_t max_num_steps,
const size_t max_ram_MB,
const double* t_eval,
Expand Down Expand Up @@ -202,7 +203,7 @@ void CySolverResult::build_solver(
this->solver_uptr = std::make_unique<RK23>(
// Common Inputs
diffeq_ptr, this->shared_from_this(), t_start, t_end, y0_ptr,
this->num_y, this->num_extra, args_ptr, max_num_steps, max_ram_MB,
this->num_y, this->num_extra, args_ptr, size_of_args, max_num_steps, max_ram_MB,
this->capture_dense_output, t_eval, len_t_eval, pre_eval_func,
// RK Inputs
rtol, atol, rtols_ptr, atols_ptr, max_step_size, first_step_size
Expand All @@ -212,8 +213,8 @@ void CySolverResult::build_solver(
// RK45
this->solver_uptr = std::make_unique<RK45>(
// Common Inputs
diffeq_ptr, this->shared_from_this(), t_start, t_end, y0_ptr, num_y,
this->num_extra, args_ptr, max_num_steps, max_ram_MB,
diffeq_ptr, this->shared_from_this(), t_start, t_end, y0_ptr,
this->num_y, this->num_extra, args_ptr, size_of_args, max_num_steps, max_ram_MB,
this->capture_dense_output, t_eval, len_t_eval, pre_eval_func,
// RK Inputs
rtol, atol, rtols_ptr, atols_ptr, max_step_size, first_step_size
Expand All @@ -223,8 +224,8 @@ void CySolverResult::build_solver(
// DOP853
this->solver_uptr = std::make_unique<DOP853>(
// Common Inputs
diffeq_ptr, this->shared_from_this(), t_start, t_end, y0_ptr, num_y,
this->num_extra, args_ptr, max_num_steps, max_ram_MB,
diffeq_ptr, this->shared_from_this(), t_start, t_end, y0_ptr,
this->num_y, this->num_extra, args_ptr, size_of_args, max_num_steps, max_ram_MB,
this->capture_dense_output, t_eval, len_t_eval, pre_eval_func,
// RK Inputs
rtol, atol, rtols_ptr, atols_ptr, max_step_size, first_step_size
Expand Down
1 change: 1 addition & 0 deletions CyRK/cy/cysolution.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ class CySolverResult : public std::enable_shared_from_this<CySolverResult>{
// General optional arguments
const size_t expected_size,
const void* args_ptr,
const size_t size_of_args,
const size_t max_num_steps,
const size_t max_ram_MB,
const double* t_eval,
Expand Down
10 changes: 9 additions & 1 deletion CyRK/cy/cysolve.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ void baseline_cysolve_ivp_noreturn(
const size_t expected_size,
const size_t num_extra,
const void* args_ptr,
const size_t size_of_args,
const size_t max_num_steps,
const size_t max_ram_MB,
const bool dense_output,
Expand Down Expand Up @@ -76,6 +77,7 @@ void baseline_cysolve_ivp_noreturn(
// General optional arguments
expected_size,
args_ptr,
size_of_args,
max_num_steps,
max_ram_MB,
t_eval,
Expand Down Expand Up @@ -103,6 +105,7 @@ std::shared_ptr<CySolverResult> baseline_cysolve_ivp(
const size_t expected_size,
const size_t num_extra,
const void* args_ptr,
const size_t size_of_args,
const size_t max_num_steps,
const size_t max_ram_MB,
const bool dense_output,
Expand Down Expand Up @@ -146,6 +149,7 @@ std::shared_ptr<CySolverResult> baseline_cysolve_ivp(
expected_size,
num_extra,
args_ptr,
size_of_args,
max_num_steps,
max_ram_MB,
dense_output,
Expand Down Expand Up @@ -192,7 +196,6 @@ PySolver::PySolver(
// General optional arguments
const size_t expected_size,
const size_t num_extra,
const void* args_ptr,
const size_t max_num_steps,
const size_t max_ram_MB,
const bool dense_output,
Expand All @@ -216,6 +219,10 @@ PySolver::PySolver(
// We also need to pass a fake pre-eval function
PreEvalFunc pre_eval_func = nullptr;

// Args are handled by the python class too.
void* args_ptr = nullptr;
size_t size_of_args = 0;

// Build the solver class. This must be heap allocated to take advantage of polymorphism.
// Setup solver class
if (this->solution_sptr) [[likely]]
Expand All @@ -229,6 +236,7 @@ PySolver::PySolver(
// General optional arguments
expected_size,
args_ptr,
size_of_args,
max_num_steps,
max_ram_MB,
t_eval,
Expand Down
1 change: 0 additions & 1 deletion CyRK/cy/cysolve.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@ class PySolver
// General optional arguments
const size_t expected_size,
const size_t num_extra,
const void* args_ptr,
const size_t max_num_steps,
const size_t max_ram_MB,
const bool dense_output,
Expand Down
21 changes: 20 additions & 1 deletion CyRK/cy/cysolver.cpp
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#include <cstring>

#include "cysolver.hpp"
#include "dense.hpp"
#include "cysolution.hpp"
Expand Down Expand Up @@ -36,6 +38,7 @@ CySolverBase::CySolverBase(
const size_t num_y,
const size_t num_extra,
const void* args_ptr,
const size_t size_of_args,
const size_t max_num_steps,
const size_t max_ram_MB,
const bool use_dense_output,
Expand All @@ -44,7 +47,6 @@ CySolverBase::CySolverBase(
PreEvalFunc pre_eval_func) :
t_start(t_start),
t_end(t_end),
args_ptr(args_ptr),
diffeq_ptr(diffeq_ptr),
len_t_eval(len_t_eval),
num_extra(num_extra),
Expand All @@ -60,6 +62,17 @@ CySolverBase::CySolverBase(
// Setup storage
this->storage_sptr->update_message("CySolverBase Initializing.");

// Build storage for args
if (args_ptr && (size_of_args > 0))
{
// Allocate memory for the size of args.
// Store void pointer to it.
this->args_ptr = malloc(size_of_args);

// Copy over contents of arg
std::memcpy(this->args_ptr, args_ptr, size_of_args);
}

// Check for errors
if (this->num_y == 0)
{
Expand Down Expand Up @@ -158,6 +171,12 @@ CySolverBase::~CySolverBase()
{
this->storage_sptr.reset();
}

// Release args data
if (args_built)
{
free(this->args_ptr);
}
}

// Protected methods
Expand Down
4 changes: 3 additions & 1 deletion CyRK/cy/cysolver.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ class CySolverBase {
const size_t num_y,
const size_t num_extra,
const void* args_ptr,
const size_t size_of_args,
const size_t max_num_steps,
const size_t max_ram_MB,
const bool use_dense_output,
Expand Down Expand Up @@ -105,7 +106,8 @@ class CySolverBase {
size_t max_num_steps = 0;

// Differential equation information
const void* args_ptr = nullptr;
void* args_ptr = nullptr;
bool args_built = false;
DiffeqFuncType diffeq_ptr = nullptr;

// t_eval information
Expand Down
11 changes: 11 additions & 0 deletions CyRK/cy/cysolver_api.pxd
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ cdef extern from "cysolution.cpp" nogil:
const int method,
const size_t expected_size,
const void* args_ptr,
const size_t size_of_args,
const size_t max_num_steps,
const size_t max_ram_MB,
const double* t_eval,
Expand Down Expand Up @@ -159,6 +160,7 @@ cdef extern from "cysolver.cpp" nogil:
const size_t num_y,
const size_t num_extra,
const void* args_ptr,
const size_t size_of_args,
const size_t max_num_steps,
const size_t max_ram_MB,
const cpp_bool use_dense_output,
Expand Down Expand Up @@ -209,6 +211,7 @@ cdef extern from "rk.cpp" nogil:
const size_t num_y,
const size_t num_extra,
const void* args_ptr,
const size_t size_of_args,
const size_t max_num_steps,
const size_t max_ram_MB,
const cpp_bool use_dense_output,
Expand Down Expand Up @@ -240,6 +243,7 @@ cdef extern from "rk.cpp" nogil:
const size_t num_y,
const size_t num_extra,
const void* args_ptr,
const size_t size_of_args,
const size_t max_num_steps,
const size_t max_ram_MB,
const cpp_bool use_dense_output,
Expand Down Expand Up @@ -267,6 +271,7 @@ cdef extern from "rk.cpp" nogil:
const size_t num_y,
const size_t num_extra,
const void* args_ptr,
const size_t size_of_args,
const size_t max_num_steps,
const size_t max_ram_MB,
const cpp_bool use_dense_output,
Expand Down Expand Up @@ -294,6 +299,7 @@ cdef extern from "rk.cpp" nogil:
const size_t num_y,
const size_t num_extra,
const void* args_ptr,
const size_t size_of_args,
const size_t max_num_steps,
const size_t max_ram_MB,
const cpp_bool use_dense_output,
Expand Down Expand Up @@ -326,6 +332,7 @@ cdef extern from "cysolve.cpp" nogil:
const size_t expected_size,
const size_t num_extra,
const void* args_ptr,
const size_t size_of_args,
const size_t max_num_steps,
const size_t max_ram_MB,
const cpp_bool dense_output,
Expand All @@ -349,6 +356,7 @@ cdef extern from "cysolve.cpp" nogil:
const size_t expected_size,
const size_t num_extra,
const void* args_ptr,
const size_t size_of_args,
const size_t max_num_steps,
const size_t max_ram_MB,
const cpp_bool dense_output,
Expand Down Expand Up @@ -377,6 +385,7 @@ cdef void cysolve_ivp_noreturn(
double rtol = *,
double atol = *,
void* args_ptr = *,
size_t size_of_args = *,
size_t num_extra = *,
size_t max_num_steps = *,
size_t max_ram_MB = *,
Expand All @@ -400,6 +409,7 @@ cdef CySolveOutput cysolve_ivp(
double rtol = *,
double atol = *,
void* args_ptr = *,
size_t size_of_args = *,
size_t num_extra = *,
size_t max_num_steps = *,
size_t max_ram_MB = *,
Expand All @@ -423,6 +433,7 @@ cdef CySolveOutput cysolve_ivp_gil(
double rtol = *,
double atol = *,
void* args_ptr = *,
size_t size_of_args = *,
size_t num_extra = *,
size_t max_num_steps = *,
size_t max_ram_MB = *,
Expand Down
6 changes: 6 additions & 0 deletions CyRK/cy/cysolver_api.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ cdef void cysolve_ivp_noreturn(
double rtol = 1.0e-3,
double atol = 1.0e-6,
void* args_ptr = NULL,
size_t size_of_args = 0,
size_t num_extra = 0,
size_t max_num_steps = 0,
size_t max_ram_MB = 2000,
Expand All @@ -131,6 +132,7 @@ cdef void cysolve_ivp_noreturn(
expected_size,
num_extra,
args_ptr,
size_of_args,
max_num_steps,
max_ram_MB,
dense_output,
Expand All @@ -154,6 +156,7 @@ cdef CySolveOutput cysolve_ivp(
double rtol = 1.0e-3,
double atol = 1.0e-6,
void* args_ptr = NULL,
size_t size_of_args = 0,
size_t num_extra = 0,
size_t max_num_steps = 0,
size_t max_ram_MB = 2000,
Expand All @@ -176,6 +179,7 @@ cdef CySolveOutput cysolve_ivp(
expected_size,
num_extra,
args_ptr,
size_of_args,
max_num_steps,
max_ram_MB,
dense_output,
Expand All @@ -201,6 +205,7 @@ cdef CySolveOutput cysolve_ivp_gil(
double rtol = 1.0e-3,
double atol = 1.0e-6,
void* args_ptr = NULL,
size_t size_of_args = 0,
size_t num_extra = 0,
size_t max_num_steps = 0,
size_t max_ram_MB = 2000,
Expand All @@ -224,6 +229,7 @@ cdef CySolveOutput cysolve_ivp_gil(
expected_size,
num_extra,
args_ptr,
size_of_args,
max_num_steps,
max_ram_MB,
dense_output,
Expand Down
Loading

0 comments on commit 4b1abd7

Please sign in to comment.