CovertLab / arrow

Stochastic simulations in python
MIT License
3 stars 1 forks source link

Avoid selection of 0 propensity reactions and negative counts #49

Closed tahorst closed 2 years ago

tahorst commented 2 years ago

This fixes #48 by avoiding reactions with a propensity of 0 when making the random choice. The issue came from the random number being exactly 0 which meant the first reaction would always be selected even if the propensity is 0 since point was 0. This caused negative counts since the reaction did not have counts to proceed which then gave a positive propensity for negative counts since they formed a dimer.

I've also added a check to cause a failure if we hit negative counts instead of an infinite memory sucking loop and rearranged some logic control flow that didn't quite make sense (checking choice == -1 after choice is set to 0 and incremented and now adding a break on an overflow error).

@1fish2: not sure if you wanted the arrow-hang branch committed to master yet but feel free to add here (or merge afterwards) since the test now passes.

U8NWXD commented 2 years ago

@1fish2 I don't have any release procedures for arrow, but in Vivarium Core I wrote a simple bash script to automated most of the release process. It:

It doesn't check that the tests are passing because in Vivarium Core, we expect the tests to always pass on master. I'd recommend setting up GitHub actions to make sure tests always pass on master here, but you could also put running the tests into the release script as an alternative.

I don't have any insights on test_flagella. @prismofeverything it looks like you introduced that test. If you have a chance to look at it, do you have any ideas about what's going wrong?

1fish2 commented 2 years ago

@U8NWXD Cool. I'll crib from that script. Borealis has a simpler script + manual steps including uploading to PyPI in case one wants to decide if it's ready to release.

1fish2 commented 2 years ago

@U8NWXD test_memory fails when run by itself and sometimes when run within test_arrow. It's trying to catch native malloc leaks with psutil.Process(os.getpid()).memory_info().rss.

Did you find a workable technique in the swiglpk memory experiments? Does tracemalloc trace allocations in C code?

U8NWXD commented 2 years ago

@1fish2 For my swiglpk debugging, I used memory_profiler. It seemed to record allocations from swiglpk, but it also didn't show all the deallocations I expected. I never managed to untangle whether that was just how Python's garbage collector worked or whether there was a C-level memory leak