Closed tirthasheshpatel closed 3 years ago
This looks great @tirthasheshpatel ! If you've verified that the scipy UNU.RAN test pass with these changes (and no crashes, etc.), I think it's good to merge and update the submodule reference.
Have you tried running any tests/examples of the SciPy wrappers with valgrind? That might help identify what the issues actually are
Thanks for looking into this, @mckib2! I verified that this builds with SciPy and also ran valgrind on simple test scripts. Thankfully, it doesn't report any leaks in UNU.RAN or the wrapper!
I might want to take back what I said earlier. Apparently, when I run valgrind with pytest scipy/stats/tests/test_sampling.py
, it gives me lots of errors in UNU.RAN :(
==25301== LEAK SUMMARY:
==25301== definitely lost: 13,728 bytes in 58 blocks
==25301== indirectly lost: 43,520 bytes in 387 blocks
==25301== possibly lost: 166,439 bytes in 114 blocks
==25301== still reachable: 3,553,979 bytes in 2,254 blocks
==25301== suppressed: 0 bytes in 0 blocks
I ran valgrind again after removing all the nonlocal returns and it reported no leaks. Turns out this is more complicated than I thought. I will have to rethink a way to handle errors. Sorry for all the mess!! Closing this and marking the PR on scipy draft again until this is done.
@mckib2
scipy/scipy#14215
I have added a Python script to help find memory leaks in a window under UNU.RAN checks. Before applying these changes, it reported around 50 leaks. Now, it reports around 18 leaks with window size 6 but most of them are:
I have fixed all the leaks under modules
discr
,distribution
,methods
,tests
,parser
, andutils
. We currently don't use much of this code but we should be able to safely use it in the future if we want to with there changes.Run the Python script to get all the errors:
python3 find_memory_leaks.py -v --window-size 6
To store the output in a file, do:python3 find_memory_leaks.py --window-size 6 --output leaks.txt