search

bowman-lab / enspara

Modeling molecular ensembles with scalable data structures and parallel computing
https://enspara.readthedocs.io
GNU General Public License v3.0
33 stars 16 forks source link

Updated synthetic_traj to use rng.choice; this doubles its speed. #211

Closed lgsmith closed 2 years ago

lgsmith commented 2 years ago

Really simple change to using choice rather than calling where on the output of multinomial. Also uses the more modern generator style of random calls that is recommended for future applications, and will draw in new entropy every time synthetic_traj is called (but not each time the method to the generator, in this case choice is called).

In my testing (just done with timeit in a scratch file with an 8000 center MSM) this change doubles the performance of this function.

justinrporter commented 2 years ago

Wow this is really slick! Do you need anything to merge it?

lgsmith commented 2 years ago

I don't think so. AFAIK I don't have maintainer status, so I can't do it myself, but if you approve it should be safe to merge. I can't see how the change could affect anything that depends on that function. The one thing that might be relevant is the use of np.random.default_rng(), while preferred in current NP code, was introduced in 1.17 (I believe). So merging this might increase the minimum workable numpy version. Other pending changes also will use generators rather than calling functions from the global generator, so I think this requirement bump is kind of unavoidable long term.

justinrporter commented 2 years ago

I think I gave you push access!

We should also change install_requires in setup.py to be something like 'numpy>=1.17', and make sure you can still install it and whatnot.

Numpy 1.17 may require a higher lowest version of python, too.

lgsmith commented 2 years ago

Thanks for the tag and the access Justin. I'll merge this now.