Refactor noise #26
Refactor noise #26
Conversation
Fixed a typo in subcase turbulence.
Updated deprecation warning text slightly.
Fixed typos.
EPR/Library/tests/test_noise.cpp
Outdated
| } | ||
|
|
||
| TEST_CASE("Testing noise: Simple test"){ | ||
| srand(10); |
There was a problem hiding this comment.
+1 avoiding rand() and srand()
There was a problem hiding this comment.
I will need to use srand() since the c version uses rand(). This make sure that random sequence will be the same when running the test for the c version (and also the c++ version). It is hard to test randomness without it.
|
I think that part of updating this module would be upgrading from rand() to random_device. If you pass a seed around, will a random_device seeded in noise.cpp generate the same sequence if you use the same seed in the test file? |
I'm confused why we would be wanting to generate the "same" randomness. We can determine if the same quality of random comes out? relevant SO |
There was a problem hiding this comment.
I defer to the open review by @dgpalmieri. This review is closed.
Delete Makeray.txt
My thought was that that would make it easy to test, but if there's a better way (that is reasonable in our timeframe), I completely agree that testing that we can generate two identical random sequences is not exactly testing that the numbers are actually "random". |
I agree that seeding random makes testing easier |
Refactored noise to c++17:
noise.c and noise.h are converted into cpp and hpp files. Noise is made into a class with the functions encapsulated as member functions. The initnoise() function is converted into a constructor for Noise.
Alias functions:
Alias free (global) functions, initnoise(), noise(), Dnoise(), turbulence(), are created to provide backward compatibility to older codes that call these functions, without needing to use the dot method. These functions are marked as deprecated.
Testing:
test_noise.cpp contain the test for both the c and c++17 version of noise. The test uses rand(), which can lead to different outcome depending on the compiler, even though seeded. The outcomes should be the same for both c and c++17 versions, ensuring that nothing changes from c to c++17. To make the test work, run the test on c version, change the number so the test passes for c, then run the test again on c++17 version, which should also pass.