Merge pull request #79 from jrenaud90/Fixing-arg-pointer-that-could-die

Fixing arg pointer that could die

.github/workflows/push_tests_ubun.yml

@@ -28,6 +28,12 @@ jobs:
       - name: Install package
         run: |
           python -m pip install . -v
+        
+      - name: Manual Test
+        run: |
+          cd Tests/E_CySolver_Tests/
+          python -c "from test_a_cysolve_ivp import test_cysolve_ivp; print('Import Success!'); test_cysolve_ivp(True, False, False, False, False, 1, 0.0, 10_000, True); print('DONE!')"
+          cd ../..
 
       - name: Run pytest
         run: pytest -n auto -v Tests/

CHANGES.md

@@ -2,7 +2,29 @@
 
 ## 2024
 
-#### v0.11.4 (2024-11-27)
+#### v0.12.0 (2024-NNN)
+
+New & Changes:
+* `MAX_STEP` can now be cimported from the top-level of CyRK: `from CyRK cimport MAX_STEP`
+* Changed uses of `void*` to `char*` for both diffeq additional args and pre-eval outputs. The signature of these functions has changed, please review documentation for correct usage.
+* Added new diagnostic info display tool to `cysolve_ivp` and `pysolve_ivp` output that you can access with `<result>.print_diagnostics()`.
+
+Fixes:
+* 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.
+* Fixed issue with diffeq test 5.
+  * This fixes GitHub Issue [#67](https://github.com/jrenaud90/CyRK/issues/67)
+* Added tests to solve large number of diffeqs simultaneously to try to catch issues related to GitHub issue [#78](https://github.com/jrenaud90/CyRK/issues/78).
+
+Documentation:
+* Updated the "Advanced CySolver.md" documentation that was out of date.
+
+#### v0.11.5 (2024-11-27)
 
 New:
 * Added a `steps_taken` tracking variable to the C++ class `CySolverResult` and the Cython wrapped `WrapCySolverResult` so that users can see how many steps were taken during integration even when using `t_eval`.

CyRK/__init__.pxd

@@ -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

CyRK/cy/common.hpp

@@ -38,9 +38,9 @@ static constexpr double SIZE_MAX_DBL = 0.99 * SIZE_MAX;
 
 
 
-typedef void (*PreEvalFunc)(void*, double, double*, const void*);
+typedef void (*PreEvalFunc)(char*, double, double*, char*);
 
-typedef void (*DiffeqFuncType)(double*, double, double*, const void*, PreEvalFunc);
+typedef void (*DiffeqFuncType)(double*, double, double*, char*, PreEvalFunc);
 
 
 

CyRK/cy/cysolution.cpp

@@ -168,14 +168,14 @@ void CySolverResult::build_solver(
         DiffeqFuncType diffeq_ptr,
         const double t_start,
         const double t_end,
-        const double* y0_ptr,
+        const double* const y0_ptr,
         const int method,
         // General optional arguments
         const size_t expected_size,
-        const void* args_ptr,
+        const char* args_ptr,
+        const size_t size_of_args,
         const size_t max_num_steps,
         const size_t max_ram_MB,
-        const bool dense_output,
         const double* t_eval,
         const size_t len_t_eval,
         PreEvalFunc pre_eval_func,
@@ -203,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
@@ -213,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
@@ -224,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

CyRK/cy/cysolution.hpp

@@ -108,14 +108,14 @@ class CySolverResult : public std::enable_shared_from_this<CySolverResult>{
         DiffeqFuncType diffeq_ptr,
         const double t_start,
         const double t_end,
-        const double* y0_ptr,
+        const double* const y0_ptr,
         const int method,
         // General optional arguments
         const size_t expected_size,
-        const void* args_ptr,
+        const char* args_ptr,
+        const size_t size_of_args,
         const size_t max_num_steps,
         const size_t max_ram_MB,
-        const bool dense_output,
         const double* t_eval,
         const size_t len_t_eval,
         PreEvalFunc pre_eval_func,

CyRK/cy/cysolve.cpp

@@ -1,6 +1,7 @@
 #include "cysolve.hpp"
 #include <exception>
 
+
 void baseline_cysolve_ivp_noreturn(
         std::shared_ptr<CySolverResult> solution_sptr,
         DiffeqFuncType diffeq_ptr,
@@ -11,7 +12,8 @@ void baseline_cysolve_ivp_noreturn(
         // General optional arguments
         const size_t expected_size,
         const size_t num_extra,
-        const void* args_ptr,
+        const char* args_ptr,
+        const size_t size_of_args,
         const size_t max_num_steps,
         const size_t max_ram_MB,
         const bool dense_output,
@@ -31,6 +33,7 @@ void baseline_cysolve_ivp_noreturn(
     const double t_start       = t_span_ptr[0];
     const double t_end         = t_span_ptr[1];
     const bool direction_flag  = t_start <= t_end ? true : false;
+    const bool forward = direction_flag == true;
     const bool t_eval_provided = t_eval ? true : false;
 
     // Get new expected size
@@ -76,9 +79,9 @@ void baseline_cysolve_ivp_noreturn(
         // General optional arguments
         expected_size,
         args_ptr,
+        size_of_args,
         max_num_steps,
         max_ram_MB,
-        dense_output,
         t_eval,
         len_t_eval,
         pre_eval_func,
@@ -103,7 +106,8 @@ std::shared_ptr<CySolverResult> baseline_cysolve_ivp(
         // General optional arguments
         const size_t expected_size,
         const size_t num_extra,
-        const void* args_ptr,
+        const char* args_ptr,
+        const size_t size_of_args,
         const size_t max_num_steps,
         const size_t max_ram_MB,
         const bool dense_output,
@@ -147,6 +151,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,
@@ -193,7 +198,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,
@@ -217,25 +221,29 @@ 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.
+    const char* args_ptr      = nullptr;
+    const 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]]
     {
         this->solution_sptr->build_solver(
-            diffeq_ptr,
+            diffeq_ptr,  // not used when using pysolver
             t_start,
             t_end,
             y0_ptr,
             integration_method,
             // General optional arguments
             expected_size,
             args_ptr,
+            size_of_args,
             max_num_steps,
             max_ram_MB,
-            dense_output,
             t_eval,
             len_t_eval,
-            pre_eval_func,
+            pre_eval_func,  // not used when using pysolver
             // rk optional arguments
             rtol,
             atol,

CyRK/cy/cysolve.hpp

@@ -15,7 +15,7 @@ void baseline_cysolve_ivp_noreturn(
     // General optional arguments
     const size_t expected_size,
     const size_t num_extra,
-    const void* args_ptr,
+    const char* args_ptr,
     const size_t max_num_steps,
     const size_t max_ram_MB,
     const bool dense_output,
@@ -38,22 +38,22 @@ std::shared_ptr<CySolverResult> baseline_cysolve_ivp(
     const size_t num_y,
     const int method,
     // General optional arguments
-    const size_t expected_size = 0,
-    const size_t num_extra = 0,
-    const void* args_ptr = nullptr,
-    const size_t max_num_steps = 0,
-    const size_t max_ram_MB = 2000,
-    const bool dense_output = false,
-    const double* t_eval = nullptr,
-    const size_t len_t_eval = 0,
-    PreEvalFunc pre_eval_func = nullptr,
+    const size_t expected_size,
+    const size_t num_extra,
+    const char* args_ptr,
+    const size_t max_num_steps,
+    const size_t max_ram_MB,
+    const bool dense_output,
+    const double* t_eval,
+    const size_t len_t_eval,
+    PreEvalFunc pre_eval_func,
     // rk optional arguments
-    const double rtol = 1.0e-3,
-    const double atol = 1.0e-6,
-    const double* rtols_ptr = nullptr,
-    const double* atols_ptr = nullptr,
-    const double max_step_size = MAX_STEP,
-    const double first_step_size = 0.0
+    const double rtol,
+    const double atol,
+    const double* rtols_ptr,
+    const double* atols_ptr,
+    const double max_step_size,
+    const double first_step_size
 );
 
 
@@ -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,

CyRK/cy/cysolver.cpp

@@ -35,7 +35,8 @@ CySolverBase::CySolverBase(
         const double* const y0_ptr,
         const size_t num_y,
         const size_t num_extra,
-        const void* const args_ptr,
+        const char* 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,
@@ -44,8 +45,8 @@ CySolverBase::CySolverBase(
         PreEvalFunc pre_eval_func) :
             t_start(t_start),
             t_end(t_end),
-            args_ptr(args_ptr),
             diffeq_ptr(diffeq_ptr),
+            size_of_args(size_of_args),
             len_t_eval(len_t_eval),
             num_extra(num_extra),
             use_dense_output(use_dense_output),
@@ -60,6 +61,22 @@ CySolverBase::CySolverBase(
     // Setup storage
     this->storage_sptr->update_message("CySolverBase Initializing.");
 
+    // Build storage for args
+    if (args_ptr && (this->size_of_args > 0))
+    {
+        // Allocate memory for the size of args.
+        // Store void pointer to it.
+        this->args_char_vec.resize(this->size_of_args);
+
+        // Copy over contents of arg
+        this->args_ptr = this->args_char_vec.data();
+        std::memcpy(this->args_ptr, args_ptr, this->size_of_args);
+    }
+    else
+    {
+        this->args_ptr = nullptr;
+    }
+
     // Check for errors
     if (this->num_y == 0)
     {
@@ -152,6 +169,12 @@ CySolverBase::~CySolverBase()
         // Decrease reference count on the cython extension class instance
         Py_XDECREF(this->cython_extension_class_instance);
     }
+
+    // Reset shared pointers
+    if (this->storage_sptr)
+    {
+        this->storage_sptr.reset();
+    }
 }
 
 // Protected methods