Display estimation of total number of nodes

[Thierry61]

In RT print routine, compute average of section length multiplied by 2 ^ section prefix length.
This gives an estimation of the total number of nodes in the network.
Display also the potential deviation given by max and min values.

This implements this idea proposed in this post in safe forum.

[maidsafe-highfive]

Thanks for the pull request, and welcome! The MaidSafe team is excited to review your changes, and you should hear from @afck (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. The way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see CONTRIBUTOR.md for more information.

[michaelsproul]

Hi @Thierry61, thanks for the pull request! I'll be reviewing your changes tomorrow morning (Australian time).

[Thierry61]

For information, here are the results on a local test network. I added nodes by group of ten until it reached 81 nodes. Then I removed nodes by group of ten also.

Real node count	Estimation
11	11 ± 0
21	21 ± 0
31	31 ± 0
41	42 ± 10
51	54 ± 6
61	64 ± 0
71	74 ± 2
81	81 ± 11
71	73 ± 15
61	64 ± 0
51	54 ± 6
41	42 ± 10
31	31 ± 0

Note that:

• estimated value is not bad at all, but estimated imprecision can be too high, or too small.

• estimations 11 ± 0, 21 ± 0 and 31 ± 0 are exact because local RT covers the whole xor space. This is true for small networks with partitions like (), 0/1, 0/10/11 or 00/01/1

• estimation 64 ± 0 is not exact (the real number of vaults was 61). It appeared by chance because the 3 sections in my RT (00/01/11) had 16 nodes each and so extrapolation to the whole network is 64 (the formula gives (16 + 16 + 16) / 3 * 2 ^ 2 = 48 / 3 * 4 = 16 * 4 = 64). But the fourth section (10) had 13 nodes, hence 61 real total nodes.

[michaelsproul]

Hi again @Thierry61,

I've been thinking about network size estimation and I have a few observations:

• I think your metric gives a reasonable estimate of the size of the network - better than anything I've been able to come up with. I tried to prove that your metric always exceeds the number of nodes in our routing table (which is desirable), but I couldn't come up with a proof. There might be a neat proof, but I'm pretty satisfied without one (maybe you have an idea here).
• However, in general, there's no upper bound on the number of nodes in the network that we can infer from the nodes in our routing table. For example, if our section prefix is 00, we are disconnected from everything in the 11 subtree, which could be arbitrarily large.
• We can however calculate an easy lower bound by first finding all prefixes that we're disconnected from (e.g. 11, if we're in 00), and adding min_section_size to the size of our routing table for each such prefix. The logic here is that every disconnected prefix contains at least one section of minimum size.

For these reasons, I think it would be better if we omitted the "max" (upper bound) estimate, and maybe the "min" as it is currently calculated. If you're really keen, you could implement my lower bound idea from above, but I certainly won't block merging this PR on that (we can do it later). Given that, I think a message like this would be good:

Estimated vault count: {estimate}

Or, if we implement the lower bound:

At least {lower_bound} vaults on the network, estimated: {estimate} vaults.

---

Code-wise, I'd just like to request a few small changes (sorry this is turning into an essay). The main one is just that it would be better for the network estimation logic to be located outside of src/states/node.rs (which we're trying to slim down, at 2K+ lines). A method called network_size_estimate on RoutingTable in src/routing_table/mod.rs would be preferable. Other than that, I noticed a few places where we could use sum rather than an explicit fold. I've commented on them in the source.

[michaelsproul]

We could use .sum() here

[michaelsproul]

Another spot we could use .sum()

[Thierry61]

I live in the opposite side of the planet (in France) so there are some delays in our communication due to time zone difference. Besides I don’t have a lot time to work on this (a few hours in the evening and on weekends). So, don’t expect an immediate delivery.

Extracting the code from node.rs is good idea, but I think that putting it in an independent file would be an even better one because src/routing_table/mod.rs is already very large. Currently my code is very short but it can grow with addition of tests and computation of lower bound (possibly). I propose network_size.rs file with an enum named NetworkSize build from a section map (output of self.peer_mgr.pub_ids_by_section()) and implementing the Debug trait (so that a NetworkSize object can be inserted in format string with {:?}).

I think it would useful to display 2 kinds of messages depending on whether the RT covers the whole XOR space:

Exact vault count: {exact}

or

Estimated vault count: {estimate}

Later the second message could be completed like you suggested with the lower bound.

I will wait for your approval and then try to deliver the minimal network_size.rs the following evening (without regression tests and lower bounds). For the rest, we will see later.

[michaelsproul]

A minimal definition of NetworkSize in a new network_size.rs file sounds great. If you don't have time to do it, I'm happy to pull your code in and update it accordingly (whilst giving you proper credit of course).

The size estimate function can take a SectionMap or a Sections<XorName>, as returned by RoutingTable::all_sections().

We're planning to do some more stuff with metrics and stats soon, so it's alright to keep this change small.

Thanks again 😄

[afck]

I would prefer to keep it as simple as possible and I do think that it can be somewhat improved and simplified at the same time, and would make sense as a method of RoutingTable:

I suggest just using
<known nodes> / <known fraction of the name space>
as an estimate. The former is routing_table.len() + 1 and the latter is the sum of all 1.0 / (1 << section_prefix.bit_count()).
Then we don't need to distinguish the two cases (if we know every node, the divisor is 1) and we are guaranteed that the estimate is always >= routing_table.len() + 1. I would also vote for omitting the ± as long as it is clear from the logs that this is just an estimate.

[Thierry61]

OK for a method of RoutingTable.

OK for removing ± (and that was already agreed with Michael)

But {known nodes} / {known fraction of the name space} seems obscure for users. And stating that the result is exact when we can, is a very useful bonus. For instance when people try to start a community network having a clear status of the network would be helpful. I think that this condition is true up to about 40..50 node. Above that an estimate is enough.

And be aware that the estimate formula can give the wrong result, even though we are in the condition to compute the exact value. Also the code is very short, it would a pity to not exploit it.

So what about something explicit like:

Exact total node count: {exact}, known fraction: {RT_size}

(here {RT_size} == {exact})

And

Estimated total node count: {estimate}, known fraction: {RT_size}

(here {RT_size} < {estimate}, I am confident on this, but I have no proof)

I replaced vault by node? And I added total?

All this replacing “Routing Table size: {:3}”? on one line instead of 2 like I did?

[afck]

I don't think my proposal can give the wrong number: If the prefixes cover the whole name space, the divisor is 1 and the dividend is the exact number of nodes in the network. And I wouldn't print the formula in the logs, so it wouldn't be confusing users.
The estimate would be exact if and only if it equals the routing table size + 1.
And it would be even fewer lines of code.

[Thierry61]

Consider a network with 40 nodes and 3 sections: 0, 10, 11. The RT contains these 3 sections and covers the whole network. There are many cases where the estimated formula doesn’t return the exact value. Here are 2 examples, with realistic values (10 and 11 having about the same number of nodes and, 0 having about twice as much as each of them):

Prefix 0	Prefix 10	Prefix 11	Estimated
23	8	9	38
17	11	12	42

And 41 exact nodes in these 3 sections with 38 estimated nodes is a real case I came across in a past test (but I didn’t write down the nodes distribution).

Frankly, the extra code is worth to add, and it’s only a few lines.

[afck]

Sorry, there's probably a misunderstanding. The formula I meant would apply as follows:

In the first example, the network size equals the routing table size plus one, which is 23 + 8 + 9 + 1 = 41.
The known fraction of the name space is 1.0 / (1 << 1) + 1.0 / (1 << 2) + 1.0 / (1 << 2) = 1 / 2 + 1 / 4 + 1 / 4 = 1 (because the prefixes have lengths 1, 2 and 2).
So the estimate would be 41 / 1 = 41, which is exact.

The same applies to the second example, except that the network size is 44 and the estimate is 44 / 1 = 44.

[Thierry61]

Just a clarification, in my examples 40 was the total number of nodes, so routing table size was 39 but your formula still applies: (39 + 1) / 1 = 40.

(41 total nodes was a real network test but the table with 40 total nodes was made with Excel)

And you’re right, there was a misunderstanding: I didn’t understand that you proposed another formula.

Now I understand it, but I would like a confirmation that it gives better results. Don’t talk about simplicity, I consider mine very simple, so it is important to implement the best formula.

Concerning displayed information how about routing_table.len() + 1 / routing_table .network_size_estimate() ? That way there is no obscure divisor. For example:

Known nodes in the network: 40 / 54

Label “Known nodes in the network” is only a proposal, so please, indicate to me the label to display.

[Thierry61]

I am thinking, that I don’t have enough time, now and the next few days and I don’t want to slow you down, so please go ahead and reimplement it the way you want.

[michaelsproul]

No worries @Thierry61, I've implemented @afck's formula in #1358

[afck]

Sorry for dragging out that discussion! But Michael implemented it already anyway, so I'll close this PR.