new GetEstimatedTimetoStake, GetEstimatedNetworkWeight (replaces GetPoSKernelPS) and GetAverageDifficulty functions #1044

jamescowens · 2018-04-05T16:42:58Z

This is intended to be responsive to issue #732.

GetPosKernelPS() has now been corrected to return more realistic netweight values and renamed to GetEstimatedNetworkWeight to more accurately reflect its function. Internally these are now in correct weight units 80 * GRC.

There is a new GetAverageDifficulty() function which returns the correct average diff.

Note both of these functions now take an optional argument of unsigned integer number of blocks to check back from current height, and default to 40 if no argument provided. The original hardcoded value of 72 everyone agreed was too long and seemingly arbitrary.

The new GetEstimatedTimetoStake function replaces the original simple formula (which never properly took into account UTXO's about to be stakeable, but not yet stakeable, among other things), and which was spread out in the code in several areas. This function also takes optional arguments of diff and the confidence level as doubles. These default to 0.0 (which is detected and processed internally as GetAverageDifficulty(40)) and 0.8 (80%) respectively.

I have replaced the denormalized calculations in the code with calls to GetEstimatedTimetoStake in both the GUI and rpc areas.

~~I have also put two new fields on the main UI status screen, Est. TTS and Est RR/day. The net weight field displays net weight in units of GRC, which is the internal net weight / 80.~~ (See the comments towards the bottom on this. The staking tooltip has been corrected but we are going to do overview UI improvements in another pr.)

Please see my comments in the code itself, and the discussion on Slack #development for additional details/observations.

Given the complexity of the algorithm, I have taken pains to make it as efficient, and take locks for as short as time possible. We need to test it on a wallet that has a really large number of UTXO's.

Note that the new ETTS algorithm gives radically different (and correct) ETTS values in the corner case of a big UTXO which is about to come off cooldown, but with only small UTXO's stakeable. The old calculation would only consider the currently stakeable (small UTXO's) sum and give a ridiculously long ETTS, when in fact the big UTXO would be stakeable very shortly and has a large probability of staking (i.e. short ETTS).

It also deals correctly with the middle ground of suboptimal UTXO count... If you have a relatively large balance, but have less than the optimal number of UTXO's, it will provide more realistic (longer in this case) ETTS values.

TheCharlatan

Could you add a brief description to your commit message? You also refactored some stuff and removed obsolete code, can you add a short list of stuff removed/refactored?

TheCharlatan · 2018-04-05T17:36:40Z

src/qt/forms/overviewpage.ui

@@ -7,7 +7,7 @@
    <x>0</x>
    <y>0</y>
    <width>948</width>
-    <height>559</height>
+    <height>942</height>


Why is this height change required?

This was done by the designer when I added the fields... Not sure if this a problem.

I wonder if this is because I have a hiDPI (4k) monitor when I was using the designer. If so, that sucks. I wonder if I can change it down manually and see what happens?

This can be changed manually, yes. And it is certainly caused by your local display.

I'll just edit it back manually. :)

TheCharlatan · 2018-04-05T17:37:41Z

src/qt/forms/overviewpage.ui

+            <item row="9" column="1">
+             <widget class="QLabel" name="labelERRperday">
+              <property name="text">
+               <string/>


You are renaming some labels to generic names like "label_4" and give proper names to others. Why?

I did this through the designer. It does that automatically, and some of the generically named fields were there before...

This code in *.ui is generated by the qt designer...

This can be modified outside of designer

Also the fields with proper names are the ones where information is plugged in via the reference to GlobalStatusStruct by the label pointers in overviewpage.cpp.

jamescowens · 2018-04-05T18:35:10Z

I changed the commit message to be more informative.

jamescowens · 2018-04-05T23:03:55Z

Paul gave me some good suggestions via slack message. I have combined the four locking sections into two and made some other tweaks. I also implemented a test for UTXO.value / 1250000 > 0, which is the same as the greater than 0.0125 GRC requirement for a UTXO to stake early in the function before the nested loop. This keeps unstakeable UTXO's out of the nested loop which reduces algorithm workload. I also manually fixed the UI form height back to what it was. The change was done by the qtdesigner and was not necessary.

jamescowens · 2018-04-06T02:57:25Z

@barton2526 made a 1000 UTXO testnet wallet for me with 1 GRC in each UTXO to test near the corner case on UTXO count. Thanks so much @barton2526!

The results for getmininginfo, which include GetEstimatedTimetoStake and several other calls..
time gridcoinresearchd-test.sh -testnet getmininginfo
real 0m0.107s
user 0m0.037s
sys 0m0.000s

The getmininginfo results for my normal testnet wallet with 11 UTXO's...
real 0m0.042s
user 0m0.024s
sys 0m0.009s

I verified via debug logs that the function is looping through all of the UTXO’s several times. Once to fill the local vector, and then several times during the nested loop. (The timings were taken with debug off though, because the output of all of that to the debug log obviously slows things...)

So that is 0.065 secs for ~1000 UTXOs. (Because the other calls in getmininginfo are the same in both. An upper bound on what is reasonable for the function without causing issues is probably 0.5 seconds.
0.5 / 0.065 * 1000 = 7692 UTXOs.

I think we are good! ;)

denravonska · 2018-04-06T17:18:44Z

src/main.cpp

        }

        pindex = pindex->pprev;
    }

-    double result = 0;
+    result = nStakesHandled ? dStakeableWeightSum / (double) nStakesTime : 0;


If there are no stakes handled this will be -1 and evaluate to true, resulting in 0/0. I suggest starting nStakesHandled at 0.

You are right I think. I will subtract one from the counter at the end, because the first trip through doesn’t actually do anything.

It will still be not zero if there are no stakes. If you make the argument unsigned and the counter start at 0 you'll be safe.

That is what I meant. I will start the counter at zero, but the final value will be one greater than the actual stakes handled...

Ok. @denravonska through Slack, before the comments above, suggested I rename GetPoSKernelPS to something better, since it doesn't affect network protocol. I also still hate the way the function calculates netweight. I tried to adapt the old calculation and I just don't like it. I am going to gut it, rename it GetEstimatedNetworkWeight and simply have it call the new function GetAverageDifficulty and use the conversion factor I already worked out to convert the average difficulty to netweight.

jamescowens · 2018-04-06T22:36:36Z

Toggleton made a pull request on my repository to add the tool tips, and I will integrate his changes too...

jamescowens · 2018-04-07T02:47:10Z

Ok. @denravonska through Slack, before the comments above, suggested I rename GetPoSKernelPS to something better, since it doesn't affect network protocol. I also still hated the way the function calculates netweight. I tried to adapt the old calculation and I just don't like it. I gutted it and replaced it with a much simpler function GetEstimatedNetworkWeight that simply calls GetAverageDifficulty and makes a conversion using a constant proportionality factor. (It should be familiar... it is the constant that comes from (MaxHash / StandardDifficultyTarget) * 16 sec / 90 sec. If you divide it by 80 to convert to GRC you get the familiar 9544517.40667.)

The last thing I have to do is put in the tool tips for the two new UI fields. --done

denravonska · 2018-04-07T03:36:58Z

src/main.cpp

            nStakesHandled++;
+            dDiff = GetDifficulty(pindex);


You can slim this down slightly to:

// dDiff should never be zero, but just in case, skip the block and move to the next one. dDiff = GetDifficulty(pindex); if (!dDiff) continue; dReciprocalDiffSum += 1 / dDiff; nStakesHandled++; if (fDebug10) LogPrintf("dDiff = %f", dDiff); if (fDebug10) LogPrintf("nStakesHandled = %u", nStakesHandled);

Ugh. Could create an infinite loop if someone specifies an interval of one, and the first block is bad. It always amazes me how the most simple things can sometimes be the most maddening in programming. I need flip the test around and get rid of the continue. If we select a bad block and exclude from the sum, we still need to do pindex = pindex->pprev to prevent the possible infinite loop.

denravonska · 2018-04-07T03:43:22Z

src/main.cpp

+        CTxIndex txindex;
+        CBlock CoinBlock; //Block which contains CoinTx
+        {
+            LOCK2(cs_main, pwalletMain->cs_wallet);


I don't think we need to lock here.

I followed what was in miner.cpp on that one. I will try without the lock and see what happens. The less we can lock the better.

Looks like it works without the lock.

denravonska · 2018-04-07T03:45:52Z

src/main.cpp

+        // Only consider UTXO's that are actually stakeable - which means that each one must be less than the available balance
+        // subtracting the reserve. Each UTXO also has to be greater than 1/80 GRC to result in a weight greater than zero in the CreateCoinStake loop,
+        // so eliminate UTXO's with less than 0.0125 GRC balances right here.
+        if(BalanceAvailForStaking >= nValue && nValue / 1250000 > 0)


&& nValue >= 1250000?

More efficient but a little more cryptic. I’ll change it.

jamescowens · 2018-04-07T16:12:42Z

I just slack messaged with @denravonska and he agreed that I should put in an assertion for the valid range of the input parameters. I am doing that now and reposting the pr.

jamescowens · 2018-04-07T19:15:55Z

That should do it unless someone sees something else...

tomasbrod

Awesome math, good refactoring, few messed up indents. I will merge locally and give it a go.

tomasbrod · 2018-04-08T11:39:25Z

src/main.cpp

+    // get the familiar 9544517.40667
+    result = 763561392.533 * GetAverageDifficulty(nPoSInterval);
+    if (fDebug10) LogPrintf("Network Weight = %f", result);
+    if (fDebug10) LogPrintf("Network Weight in GRC = %f", result / 80.0);


Missing log prefix. Or is it now handled by the logging function?

No. I missed it. I should add it and repost the commit...

There are more occurences. Btw you do not have to ammend every time, prs can take multiple commits, it's up to you.

I think I got all of them. I also changed them all to actually specify the exact function name. I was using ETTS...

tomasbrod · 2018-04-08T11:56:21Z

src/main.cpp

+    // If it was not degenerate and the positive reqions in the Gantt chart area contributed some probability, then dCumulativeProbability will
+    // be greater than zero. We must compute the amount of time beyond nTime that is required to bridge the gap between
+    // dCumulativeProbability and dConfidence. If (dConfidence - dCumulativeProbability) <= 0 then we overshot during the Gantt chart area,
+    // and we will back off by nThrows amount, which will now be negative.


Yeah... it is a bit wild, but better than looping every ETTS mask quantum to prevent overshooting. That would require 225+ loops. Most of the time there will only be a few UTXO ends “events” in the outer loop... in many instances only 1 to 3, and this reduces the loop workload both outer and inner considerably...

Also, as you noticed, the same code handles the “tail”, which is very efficient. For lower balance folks, most of the time all of their UTXO’s will be mature, and so the main loop gets iterated once with the current time to add up all of the UTXO probabilities and then this part of the code calculates the nThrows (time).

tomasbrod · 2018-04-08T12:02:28Z

src/main.cpp

+
+    // Derive a smoothed difficulty over desired PoSInterval of blocks by calling GetPoSKernelPS() which is netweight and reverse engineering...
+    double dDiff = GetAverageDifficulty(nPoSInterval);
+    if (fDebug10) LogPrintf("ETTS debug: dDiff = %f", dDiff);


Allow this difficulty to be overridden by function parameter.

Hmm... Both GetAverageDifficulty and GetEstimatedTimetoStake have default nPoSInterval arguments of 40. The intent here is to allow someone calling GetEstimatedTimetoStake a different choice of number of blocks back to look to average the Difficulty than the default 40. If I just called GetAverageDifficulty() here without the argument, it would be fixed at the 40, and I would not use the default argument nPoSInterval on GetEstimatedTimetoStake.

So if someone calls GetEstimatedTimetoStake(80, 0.63) this would send 80 on through to line 556 as GetAverageDifficulty(80), which is what we want I think...

I mean, I can see someone would want to see ETTS at arbitrary difficulty, not just the current. Instead of nPoSInterval and calculatiing the average difficulty internally, the function would take difficulty as argument.
GetEstimatedTimetoStake(GetAverageDifficulty(80), 0.63) or
GetEstimatedTimetoStake(4, 0.70)
However I am very satisfied with the pr how it is now.

I get what you are saying now. Do you want me to change it and repush the commit? It is pretty easy... Oh wait... in the header you would need...

double GetEstimatedTimetoStake(double dDifficulty = GetAverageDifficulty(40), double dConfidence = 0.8);

to allow someone to simply say GetEstimatedTimetoStake(); and mean get the average diff for the last 40 blocks and confidence of 80%. Can you specify a default value = to a function in the header?

I do not think so, but you can do double GetEstimatedTimetoStake(double dDifficulty = -1, double dConfidence = 0.8);

I did dDifficulty = 0.0, since normally specifying a dDiff of 0 is nonsensical. Makes the assertion for crazy parameters easier.

tomasbrod · 2018-04-08T16:59:22Z

src/main.cpp

+    // The old calculation for comparative purposes...
+    double oldETTS = 0;
+
+    oldETTS = GetTargetSpacing(nBestHeight) * GetEstimatedNetworkWeight(nPoSInterval) / MinerStatus.WeightSum;


Is this used except for debug? If move it inside the if.

Nope. I should move it.

jamescowens · 2018-04-08T18:32:36Z

Ok I made some tweaks based on @tomasbrod's comments and also some other minor cleanups (including removing the commented out lock we didn't need).

jamescowens · 2018-04-08T23:14:56Z

After pondering some more whether the harmonic mean and arithmetic mean is the best for averaging diff, I have decided to change it to arithmetic. I actually have created a spreadsheet which shows the response of the retargeting algorithm to when a whale staker (or several) comes online and "stakes", and then goes away. In reality the situation would be a bit more spread out and more random, but the model shows the overall effects. The arithmetic average responds better on an increasing of Diff (more coins staking than before), while the geometric responds better when Diff decreases. They are pretty similar though, so I have elected to go back to arithmetic because it is more understandable for people.

Please see the link to the google sheet if you want to examine more closely...
https://docs.google.com/spreadsheets/d/1FGJewsTIWMhxHf--GjfOOa57npoY8uC8Mpux7kURZas/edit?usp=sharing

jamescowens · 2018-04-09T15:43:26Z

The spreadsheet deserves some detailed commentary to help people interpret it. I have added two graphs to provide a visual of what is going on. This is a "simulation" of 1000 blocks and what happens to measured diff and estimated netweight when large stakers (whales) come on and off.

The upper portion of the spreadsheet includes parameters that are defined from the current code.
Column A ("Block") is the block number in the simulation. (This is not meant to represent actual block numbers, but just a consecutive sequence of blocks starting at "1" within the V9 protocol.)
Column B ("GRC Staking") is what I call the "God" column. It is the actual GRC staking in the network. Note that for the purpose of this simulation, it is an input, but this quantity is NOT directly measurable. (If it was, we wouldn't be having this discussion!)
Column C ("Difficulty") is the current Difficulty recorded in the block.
Column D ("Frequency of Stake in 90s") is the Frequency of Stakes that will occur in 90 secs given the GRC's staking. Note that in reality this would have a lot of scatter, but for the purpose of this demonstration, it is computed as a theoretical value based on the staking probability.
Column E ("Theoretical Block Spacing") is the reciprocal of Column C.
Column F ("NextDiff") This is the target value of the next block selected based on the block spacing. This formula comes directly from GetNextTargetRequiredV2. (Note that it is the reciprocal of the formula used in GetNextTargetRequiredV2, because the ratio of Diff (i+1 block) to Diff (ith block) is the reciprocal of the ratio of Target(i+1 block)/Target(ith block). You will notice that the value of Column E is used for the next block's Difficulty in Column C. This is the way the retargeting actually works!
Column G ("Arithmetic Average of Difficulty") This is the 40 block average of Difficulty using the standard arithmetic average.... sum(Difficulty over 40 blocks) / 40.
Column H ("Netweight via Arithmetic Average") is the netweight estimated by the formula netweight = 9544517.40667 * Diffculty (via Arithmetic Average).
Column I ("Harmonic Average of Difficulty") This is the 40 block average of Difficulty using the harmonic average.... 1 / (sum(1/Diff over 40 blocks) / 40).
Column J ("Netweight via Harmonic Average") is the netweight estimated by the formula netweight = 9544517.40667 * Diffculty (via Arithmetic Average).

I start the simulation with the staking population of GRC = 10000000 and an initial diff of 1.0. (This is slightly out of equilibrium. An equilibrium diff of 1.0 -> GRC of 9544517.) You will notice that the diff adjusts to the equilbrium value of 1.04768. At block 120 I raise the GRC population to 20000000, simulating a 10000000 GRC whale suddenly coming online. At block 251 I have the whale dropping out and the GRC population back to 10000000. Then at block 500 I add in a much bigger whale at 32000000 (raising the GRC population to 42000000) and then at 751 the whale drops off.

The graphs vividly show the effects. When the whales come online, the block frequency temporarily increases. The retargeting algorithm acts to raise diff to compensate, bringing the block frequency (spacing) back to the target values. The inverse happens when the whales drop off. The graph shows what happens to the measurable quantities. Interestingly the arithmetic average more quickly converges on the "truth" when the GRC goes up, and the harmonic is quicker to converge with the GRC goes down.

You can also see the lag in the response of the measurable quantities to "reality".

Corrects netweight -- replaces the old GetPosKernelPS() with a new function GetEstimatedNetworkWeight(). (The old function does not have anything to do with network protocol and was misleading.) Implements new EstimatedTimetoStake() with a better algorithm and replaces denormalized code in both GUI and RPC areas. Implements new GetAverageDifficulty(). Changes the staking tooltip to use the correct functions. The overview ui will be dealt with in a separate pr/commit.

jamescowens · 2018-04-11T18:35:35Z

Upon conferring with @denravonska, he concurs, and prefers, that we move UI work to a separate PR. I have therefore removed the overview.ui changes and only retained the corrections to the staking tooltip functions that were put in. We will open separate PR(s) to deal with the overview page UI overhaul.

jamescowens force-pushed the development branch from aeb0954 to 2d8ebdf Compare April 5, 2018 16:52

jamescowens changed the title ~~new Estimated TTS, Get GetPoSKernelPS and GetAverageDifficulty functions~~ new Estimated TTS, GetPoSKernelPS and GetAverageDifficulty functions Apr 5, 2018

TheCharlatan reviewed Apr 5, 2018

View reviewed changes

jamescowens force-pushed the development branch 2 times, most recently from 9713289 to 40afb05 Compare April 5, 2018 18:33

jamescowens force-pushed the development branch 2 times, most recently from 47c1826 to 2468aa5 Compare April 5, 2018 22:41

denravonska reviewed Apr 6, 2018

View reviewed changes

jamescowens force-pushed the development branch from 2468aa5 to aa2b49b Compare April 7, 2018 02:42

denravonska reviewed Apr 7, 2018

View reviewed changes

jamescowens changed the title ~~new Estimated TTS, GetPoSKernelPS and GetAverageDifficulty functions~~ new EstimatedTimetoStake, GetEstimatedNetworkWeight (replaces GetPoSKernelPS) and GetAverageDifficulty functions Apr 7, 2018

jamescowens force-pushed the development branch 3 times, most recently from 12d8484 to 5635827 Compare April 7, 2018 16:11

jamescowens force-pushed the development branch 2 times, most recently from 38674f4 to 59af9df Compare April 7, 2018 18:27

tomasbrod approved these changes Apr 8, 2018

View reviewed changes

jamescowens changed the title ~~new EstimatedTimetoStake, GetEstimatedNetworkWeight (replaces GetPoSKernelPS) and GetAverageDifficulty functions~~ new GetEstimatedTimetoStake, GetEstimatedNetworkWeight (replaces GetPoSKernelPS) and GetAverageDifficulty functions Apr 8, 2018

tomasbrod reviewed Apr 8, 2018

View reviewed changes

jamescowens force-pushed the development branch from 59af9df to 8acd6ae Compare April 8, 2018 18:28

jamescowens force-pushed the development branch from 8acd6ae to f14c1d8 Compare April 8, 2018 22:18

jring-o mentioned this pull request Apr 11, 2018

Constant Block Reward (CBR) Value Proposal and Poll gridcoin-community/economics#1

Open

jamescowens force-pushed the development branch from f14c1d8 to 50de265 Compare April 11, 2018 18:30

denravonska merged commit bded234 into gridcoin-community:development Apr 12, 2018

new GetEstimatedTimetoStake, GetEstimatedNetworkWeight (replaces GetPoSKernelPS) and GetAverageDifficulty functions #1044

new GetEstimatedTimetoStake, GetEstimatedNetworkWeight (replaces GetPoSKernelPS) and GetAverageDifficulty functions #1044

Conversation

jamescowens commented Apr 5, 2018 • edited Loading

TheCharlatan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jamescowens Apr 5, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jamescowens Apr 5, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jamescowens Apr 6, 2018 • edited Loading

Choose a reason for hiding this comment

jamescowens commented Apr 5, 2018 • edited Loading

jamescowens commented Apr 5, 2018 • edited Loading

jamescowens commented Apr 6, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jamescowens commented Apr 6, 2018

jamescowens commented Apr 7, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jamescowens Apr 7, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jamescowens Apr 7, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jamescowens commented Apr 7, 2018 • edited Loading

jamescowens commented Apr 7, 2018

tomasbrod left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jamescowens Apr 8, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jamescowens Apr 8, 2018 • edited Loading

Choose a reason for hiding this comment

tomasbrod Apr 8, 2018 • edited Loading

Choose a reason for hiding this comment

jamescowens Apr 8, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jamescowens commented Apr 8, 2018

jamescowens commented Apr 8, 2018 • edited Loading

jamescowens commented Apr 9, 2018 • edited Loading

jamescowens commented Apr 11, 2018

jamescowens commented Apr 5, 2018 •

edited

Loading

jamescowens Apr 5, 2018 •

edited

Loading

jamescowens Apr 5, 2018 •

edited

Loading

jamescowens Apr 6, 2018 •

edited

Loading

jamescowens commented Apr 5, 2018 •

edited

Loading

jamescowens commented Apr 5, 2018 •

edited

Loading

jamescowens commented Apr 6, 2018 •

edited

Loading

jamescowens commented Apr 7, 2018 •

edited

Loading

jamescowens Apr 7, 2018 •

edited

Loading

jamescowens Apr 7, 2018 •

edited

Loading

jamescowens commented Apr 7, 2018 •

edited

Loading

jamescowens Apr 8, 2018 •

edited

Loading

jamescowens Apr 8, 2018 •

edited

Loading

tomasbrod Apr 8, 2018 •

edited

Loading

jamescowens Apr 8, 2018 •

edited

Loading

jamescowens commented Apr 8, 2018 •

edited

Loading

jamescowens commented Apr 9, 2018 •

edited

Loading