-
Notifications
You must be signed in to change notification settings - Fork 75
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
TM, SP: fix possible underflow in seed. Default random (0) seed. #799
base: master
Are you sure you want to change the base?
Conversation
- fixed type for possible integer underflow in SP - default seed is "random" (0). Also in Python. - use directly fixed seed where needed in tests for deterministic results. (for this added c++ helper methods `setSeed(UInt)` ) - fix docs - update tests To be used by NAB detector.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dkeeney could you have a look with me please?
Most changes ironed out, but there's something stinky in the SP's seed, serialization. I recall a user reporting a bug here with different results with a SP loaded from serialized state.
@@ -94,6 +93,12 @@ EPOCHS = 2; // make test faster in Debug | |||
Metrics statsSPglobal(outSPglobal, 1000); | |||
Metrics statsTM(outTM, 1000); | |||
|
|||
//uses fixed seed for deterministic output checks: | |||
Random rnd(42); | |||
spGlobal.setSeed(1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a strange thing is happening here, I set the rng_
to fixed seed (1 used to be used before) ...but determ. output is still changing!
@@ -173,9 +173,9 @@ Argument boostStrength A number greater or equal than 0, used to | |||
too much boosting may also lead to instability of SP outputs. | |||
|
|||
|
|||
Argument seed Seed for our random number generator. If seed is < 0 | |||
Argument seed Seed for our random number generator. If seed is 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
corrected the doc strings
@@ -197,7 +197,7 @@ Argument wrapAround boolean value that determines whether or not inputs | |||
, py::arg("minPctOverlapDutyCycle") = 0.001 | |||
, py::arg("dutyCyclePeriod") = 1000 | |||
, py::arg("boostStrength") = 0.0 | |||
, py::arg("seed") = 1 | |||
, py::arg("seed") = 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed the defaults to "random" (0) in cpp, py, NetworkAPI
|
||
testTimer.stop(); | ||
|
||
EXPECT_EQ(outputBaseline, outputC); | ||
EXPECT_EQ(outputBaseline, outputC); //FIXME this test randomly fails. (De/serialization of rng_ is correct?) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is another test that is inconsistent! Sometimes it passes, sometimes crashes on different outputs. I can't see why would that be, the test is relatively simple and seems correct.
I'm suspecting something with serizalization of Random?
Even more confusing is why HellpSPTP fails undeterministically now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I took a quick look at the Random code and it looks ok. Nothing obvious that might cause it to fail.
random seed was not properly applied.
replaced by Cereal
replaced by Cereal
I need more precise detail if/why two SPs differ.
these are helper data classes that hold "heavy" info on connections: SegmentData, SynapseData
to be more descriptive when difference is found. Useful for debugging.
apparently improper combination of cereal::make_nvp() and CEERAL_NVP(). Only the latter should be used. Finished simplified Conn serialization, only data-members are directly serialized. No need to call initialize() and other functions manually.
Waiting for bigger changes in #601 , after that I should debug this. |
now passes after merging the last PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
results. (for this added c++ helper methods
setSeed(UInt)
)To be used by NAB detector.
For use in #792 ,