thoduv / pyradar5

Python wrapper for the RADAR5 DDE integrator
Other
3 stars 2 forks source link

Fixed size memory allocation limits dimensionality of the equation system. #1

Open GColom opened 3 years ago

GColom commented 3 years ago

https://github.com/thoduv/pyradar5/blob/c2c30fa8c0afa8b7d0efdc19aacdadd1e1ef092d/wrapper.c#L79-L83

When using the package to simulate systems of dimension larger than 40, I get a Bus error (Ubuntu 20.04).

By adding some print instructions in the wrapper.c file, I have been able to track this back to the PHI function (line 232, file wrapper.c), trying to access an element in one of the arrays initialised at the listed lines, this raising a bus error due to the fact that the dereferenced address actually contains the next struct variable, and not an array element (Y[40]). I am confident that this is the problem, since if one changes 10 to any other number N, and recompiles the package, the "bound" is moved to 4*N.

Since this package can prove very useful in simulating large systems, I think it would be advisable to fix this issue by implementing dynamical allocation, if possible. See also the issue https://github.com/GColom/pyradar5/issues/1 , on my fork of the repository.

thoduv commented 3 years ago

Hi,

This is indeed a hard-coded limit due to lazyness, and it should have been mentionned in the documentation. Sorry about that. You are right, the best thing to do would be to dynamically allocate the right number of initial conditions according to the size of the system. It is not hard to do, but for the time being I don't have any access to a build environment so I can't do it.

Anyway, a temporary (but effective) fix would be to extend the size of all those arrays from 10 to something large, say 1000. It will look bad, but it won't do any harm because hey, we all have a few kilobytes to waste nowadays !