Address partly a test failing due to stochasticity #129
Conversation
It needs to be run from the root of the repository
Issue: it was not easy to find that there were extra dependencies for testing Solution: mention it explicitely in the CONTRIBUTING.md
issue: the test was failing randomly when accessing whether the sampled coincidence were following a normal distribution, due to the fact we apply math.ceil to the random.gauss to get integer number of appliances to be switched on simultaneously solution: get an experimental probability density function and fit a normal distribution to it, look how large is the error of mean and std as a proxy of visual inspection of the graph
|
Hi @Bachibouzouk; thanks, the tests all worked fine for me, now! :) |
There was a problem hiding this comment.
The updated test looks good, but the -0.5 correction in calc_coincident_switch_on looks still wrong to me. I have attached two new detailed examples of what I get with or without it. I have done the test not only for the histogram, which we were skeptical about due to the subjectivity of bins' dimensions, but also for a kernel distribution of the same RAMP results. Both visualisations agree on the better performance without the -0.5 correction.
Bachibouzouk
force-pushed
the
review/failing_tests
branch
from
b6f9043 to
4559bcf
Compare
Sorry @FLomb - I had not reworked this PR since our last discussion, now I removed the correction |
Related to #99