redstreet/fava_investor

Incorrect allocation percentage computation in "Asset Allocation Accounts"

jenicek opened this issue · 4 comments

The allocation percentage is computed incorrectly when positive and negative balances are mixed in one table, see the following patch for a fix.

diff --git a/fava_investor/modules/assetalloc_account/libaaacc.py b/fava_investor/modules/assetalloc_account/libaaacc.py
index 6dea472..4607e7d 100644
--- a/fava_investor/modules/assetalloc_account/libaaacc.py
+++ b/fava_investor/modules/assetalloc_account/libaaacc.py
@@ -73,9 +73,9 @@ def asset_allocation(nodes, accapi, include_children):
             row["balance"] = balance[operating_currency]
             rows.append(row)
 
-    portfolio_total = sum(row['balance'] for row in rows)
+    portfolio_total = sum(abs(row['balance']) for row in rows)
     for row in rows:
         if "balance" in row:
-            row["allocation %"] = round((row["balance"] / portfolio_total) * 100, 1)
+            row["allocation %"] = round(abs(row["balance"] / portfolio_total) * 100, 1)
 
     return types, rows

If you could explain the rationale for how to handle negative numbers in asset allocation, that would be great.

Negative numbers can represent multiple things including liabilities for leverage, and short positions. I'm wondering if it makes sense to treat all of them the same?

I'm just learning beancount, so I cannot really tell whether it is useful to mix positive and negative numbers - I was playing with short positions and also used the table to compare incomes vs. expenses. Each is probably a corner case, so I am not sure whether it makes sense from an accounting point of view, but i believe that displaying -1450 % and 1550 % when comparing two numbers of an opposite sign is a bug either way :)

The reason why I chose to display them as negative numbers (for now) is so they alert the user to their existence as negative numbers, and don't silently do something unexpected or incorrect. Happy to keep this thread open until a definitive solution has been arrived at.

Closing this. Feel free to reopen if you have insights on good solutions.