Rogue node detection.

[CdavM]

This PR should be used to discuss an implementation of the rogue node algorithm implementation.

[mitar]

So, why are tests failing here?

[CdavM]

It's no longer failing. Had to rename a couple of modules.

[mitar]

Hm, do you think it is useful to store whole results for your tests? Maybe. But isn't the idea that you detect rogue nodes, but slight changes in values might not be so problematic? So maybe more important is that class rogue/not does not change?

[CdavM]

When you say "results", you mean the output of the detection algorithm? Meaning a bunch of nodes and the probability of each being rogue? I think we need to store all the data because that should be the output of an algorithm. An alternative is to filter the results before the algorithm outputs them, but I prefer it the way it is now. But it is true that I am assuming that there is only one rogue node in these test cases and we are not sensitive at all to perturbations.

So what should we do?

[mitar]

I mean results of tests. So you have files with results and you compare then against those files, no? Do we really want to require that those results always in the future match?

[CdavM]

Yes I think that's a good idea. So the way I see it the algorithm works fine for Cloyne but may not work for other applications. And if someone wants to improve it they should make sure it's strictly better than the one we have right now.

How else would you do it without requiring that the Cloyne inputs produce the same output?

[mitar]

Yes I think that's a good idea. So the way I see it the algorithm works fine for Cloyne but may not work for other applications. And if someone wants to improve it they should make sure it's strictly better than the one we have right now.

Yes, the detection should be strictly better, but not all numbers being output from the algorithm. Currently you require all the numbers to be the same, no?

How else would you do it without requiring that the Cloyne inputs produce the same output?

For every test case you just check that the probability of the detection of the rogue node is inside some margin, so not exact value. So if it is x = 0.6 < 0.9 (0.9 is limit for example where algorithm says it is a rogue node), then you allow 0.5 < x 0.7 < 0.9 or something. So you allow probability to fluctuate a bit, but you do not allow that it changes the classification.

But it is OK. You do not have to worry about this. I am commenting so that you think a bit about this. We can also change this in the future when we get to more tests and break these tests.

[mitar]

So, what is the status of this? Do you think it is ready for a code review?

[CdavM]

Yes, I implemented a custom test suite. Please do a code review.

[CdavM]

We could also try to ensure that the numbers are in a certain range, as you proposed, but it is not within the scope of the algorithm to determine the "classification" of a rogue node. That is done by the maintainers. So we might be breaking abstraction barriers.

But more importantly, I don't foresee anyone tampering with this algorithm in the near future so I wouldn't spend too much time trying to predict how they will work on it.

[mitar]

I reviewed the pull request. Please answer questions I made. Also see changes I made. See if everything still works.

[CdavM]

Hi, not all tests are running with your code. Django only reports running one test: comparing 'behind-couch-....' with its results. How do we fix this?

[CdavM]

@mitar Can you do a code review of this module?

[mitar]

Perfect. Merging!