Bugfix for detecting change points based on MOA #1613
Conversation
…ndex should increment from 0 to bucket.current_idx - 1. The previous code "for k in range(bucket.current_idx - 1):" only increment k to bucket.current_idx - 2 because of the range function.
…ndex should increment from 0 to bucket.current_idx - 1. The previous code "for k in range(bucket.current_idx - 1):" only increment k to bucket.current_idx - 2 because of the range function.
|
I reported the bug on issue Change points reported different from MOA - Method _detect_change #1614 |
|
#1535 and #1539 might also concern the same problem. Thanks for the contribution, @denisesato ! The open issue (#1539) did not evolve due to the lack of benchmarking, and lack of time from my side, honestly. |
There was a problem hiding this comment.
@denisesato, as a lot of learning models rely on ADWIN, the changes you introduced made some tests fail. Could you please also update them?
|
@smastelini: I have updated the tests that failed after the change on ADWIN. |
There was a problem hiding this comment.
Thank you, @denisesato for the latest changes!
It is interesting to see that changing ADWIN impacts both positively and negatively some algorithms. In any case, the doc tests use small samples for learning and I don't think they are a comprehensive benchmark.
No description provided.