j-bryan commented 10 months ago

Pull Request Description

What issue does this change request address? (Use "#" before the issue to link it, i.e., #42.)

2204

What are the significant changes in functionality due to this change request?

Allows for the use of the numpy stochastic environment by default instead of relying on Crow. The new numpy RNG produces outputs identical to the Crow RNG, with the added NumpyRNG class providing an equivalent interface as the existing CrowRNG class.

One disadvantage of this approach is that many of the nice functions that the numpy interface provides are not usable while still producing identical output due differences in how the underlying Mersenne Twister RNG engine is used when producing anything but 32-bit integers.

For Change Control Board: Change Request Review

The following review must be completed by an authorized member of the Change Control Board.

[x] 1. Review all computer code.
[x] 2. If any changes occur to the input syntax, there must be an accompanying change to the user manual and xsd schema. If the input syntax change deprecates existing input files, a conversion script needs to be added (see Conversion Scripts).
[x] 3. Make sure the Python code and commenting standards are respected (camelBack, etc.) - See on the wiki for details.
[x] 4. Automated Tests should pass, including run_tests, pylint, manual building and xsd tests. If there are changes to Simulation.py or JobHandler.py the qsub tests must pass.
[x] 5. If significant functionality is added, there must be tests added to check this. Tests should cover all possible options. Multiple short tests are preferred over one large test. If new development on the internal JobHandler parallel system is performed, a cluster test must be added setting, in XML block, the node <internalParallel> to True.
[x] 6. If the change modifies or adds a requirement or a requirement based test case, the Change Control Board's Chair or designee also needs to approve the change. The requirements and the requirements test shall be in sync.
[x] 7. The merge request must reference an issue. If the issue is closed, the issue close checklist shall be done.
[x] 8. If an analytic test is changed/added is the the analytic documentation updated/added?
[x] 9. If any test used as a basis for documentation examples (currently found in raven/tests/framework/user_guide and raven/docs/workshop) have been changed, the associated documentation must be reviewed and assured the text matches the example.

j-bryan commented 9 months ago

Some benchmarks for the numpy RNG code and scipy distributions. This compares the results of the current pull request, the current devel branch (982644d0446c6955b3ca004dbcba4736c5a81b70), and the last commit before the distributions were reimplemented in scipy (177725318819492917e2d8e1904f750cc7f2846b). These will be referred to as "feature", "devel", and "previous" here. The tests were run on my 7-year-old Dell XPS laptop.

Benchmark Times:	Test/Function	Branch	RNG	Total Samples
randomUtils.random(dim=1, size=1)	feature	numpy	1000000	54.23
randomUtils.random(dim=1, size=1)	feature	crow	1000000	23.17
randomUtils.random(dim=1, size=1)	devel	crow	1000000	31.66
randomUtils.random(dim=1, size=1)	previous	crow	1000000	23.98
-	-	-	-	-
randomUtils.random(dim=1000000, size=1)	feature	numpy	1000000	0.02
randomUtils.random(dim=1000000, size=1)	feature	numpy	1000000	0.94
randomUtils.random(dim=1000000, size=1)	feature	numpy	1000000	1.19
randomUtils.random(dim=1000000, size=1)	feature	numpy	1000000	1.27
-	-	-	-	-
randomUtils.randomNormal(size=1)	feature	numpy	1000000	92.83
randomUtils.randomNormal(size=1)	feature	crow	1000000	73.61
randomUtils.randomNormal(size=1)	devel	crow	1000000	69.30
randomUtils.randomNormal(size=1)	previous	crow	1000000	65.63
-	-	-	-	-
randomUtils.randomNormal(size=1000000)	feature	numpy	1000000	0.53
randomUtils.randomNormal(size=1000000)	feature	crow	1000000	1.75
randomUtils.randomNormal(size=1000000)	devel	crow	1000000	28.08
randomUtils.randomNormal(size=1000000)	previous	crow	1000000	27.44
-	-	-	-	-
Distributions.Normal.rvs()	feature	numpy	10000	32.33
Distributions.Normal.rvs()	feature	crow	10000	25.89
Distributions.Normal.rvs()	devel	crow	10000	28.23
Distributions.Normal.rvs()	previous	crow	10000	1.59
-	-	-	-	-
Distributions.MultivariateNormal.rvs() (dim=5)	feature	numpy	10000	37.61
Distributions.MultivariateNormal.rvs() (dim=5)	feature	crow	10000	33.83
Distributions.MultivariateNormal.rvs() (dim=5)	devel	crow	10000	1.78
Distributions.MultivariateNormal.rvs() (dim=5)	previous	crow	10000	1.61
-	-	-	-	-
tests/framework/MCMC/test_1D.xml	feature	numpy	80000	1827
tests/framework/MCMC/test_1D.xml	devel	crow	80000	1821
tests/framework/MCMC/test_1D.xml	previous	crow	80000	1800
-	-	-	-	-
tests/framework/Distributions/test_distributionsGeometric.xml	feature	numpy	10000	230
tests/framework/Distributions/test_distributionsGeometric.xml	devel	crow	10000	232
tests/framework/Distributions/test_distributionsGeometric.xml	previous	crow	10000	231

My observations:

The feature branch using the numpy RNG tends to be the slowest when generating random values one at a time but also significantly faster than the other versions when generating many values at once.
The vectorized implementation in the randomUtils.BoxMullerGenerator class on the feature branch provides significant speedups over the previous implementation for both RNG engines.
Sampling from a Distribution class has slowed down significantly with the switch to the scipy distribution implementation. This could likely be alleviated significantly by vectorizing that sampling process as well, but that will not be helpful for sampling values one at a time.
These speedups and slowdowns seem to have very little real effect on the speed of the MCMC test and the Distributions test that were benchmarked. A broader sampling of tests would be helpful to determine if this is generally true.