Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added adjusted internal rate of return #2975

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Lasall
Copy link

@Lasall Lasall commented Sep 11, 2022

Added new widget Adjusted IRR that allows to set benchmark to e.g. consumer price index to get inflation adjusted IRR.

Implementation of suggested changes in PR #1120

Didn´t name it "Inflation adjusted/Inflationsbereinigt" since it is possible to select any other benchmark. The IRR widget could possibly be merged replaced with this widget (then the title needs to adjust dynamically and the ability to remove benchmark once set). Maybe the tooltip could be extended to show individual IRRs and calculation steps.

@Lasall Lasall force-pushed the feature_adjusted_irr branch from 8cfb317 to f655f6d Compare September 14, 2022 18:33
@Lasall
Copy link
Author

Lasall commented Sep 14, 2022

Merged the changes into the existing IRR widget and allowed removal of the benchmark.

@Lasall Lasall marked this pull request as ready for review September 14, 2022 18:42
@Lasall Lasall changed the title Draft: Added adjusted internal rate of return Added adjusted internal rate of return Sep 20, 2022
- Update IRR widget to add benchmark.

  That way the inflation adjusted IRR can be displayed by selecting a
  relevant consumer price index as benchmark.
@Lasall Lasall force-pushed the feature_adjusted_irr branch from f655f6d to 3ce9954 Compare September 21, 2022 21:09
@buchen
Copy link
Member

buchen commented Oct 3, 2022

Sorry for being so late. I have been quite busy. Hope to have more time soon.

@chirlu
Copy link
Member

chirlu commented Nov 6, 2022

As I’ve said in a few places, I think the best way to handle inflation adjustment would be to treat it as a currency: “real Euro” or “Euro as of 2020”, say, with a currency code of EU0. The exchange rate between EUR and EU0 is given by the consumer price index. You could then easily change the reporting currency as desired. PP will show nominal values when EUR is selected and real values when EU0 is selected – everywhere! Every widget, every graph, every table will automatically be inflation-adjusted, without any additional work.

On the other hand, creating special inflation-adjusted versions for all widgets is redundant work and a maintenance nightmare.

@RomanLangrehr
Copy link
Contributor

@chirlu I don’t think it would be a good idea to do this with a synthetic currency for the following reasons:

  1. It is less ituitive to use, in my opinion.
  2. You can get either inflation-adjusted returns or nominal returns, not both in the same dashboard (but one might add an option to each widget to change the currency for this widget, similar to how this is done with the reporting periode).
  3. Widgets which show a monetary amount will do this also in “Euro as of 2020”, which is a quite arbitrary measure (especially when 2020 is not even within the reporting periode).
  4. People might mix up “EUR” and “EU0” in their transactions.
  5. Most imporantly: This approach is much less flexible. There is currently (as far as I know) no way to add your own currencies to PP, so one would be restricted to a set of built-in inflation indices. However, I would be very much interested in calculating returns adjusted for money market interested rates (i.e. using indices like EONIA or X-month EURIBOR instead of an inflation index) and that seems to be impossible with the currency-based approach.

However, your concern about redundancy is very valid. But I think this can be adressed by building the adjustment for an index within the PerformanceIndex class (and not within a GUI class like the IRR widget). Ideally, every widget than has to specify only whether it supports adjustment for inflation (or any other index) or not, and not provide any logic to do the calculations. If I have time, I will try if I get this idea to work.

@chirlu
Copy link
Member

chirlu commented Nov 18, 2022

@RomanLangrehr

  1. It is less ituitive to use, in my opinion.

It is more intuitive, in my opinion, to have “2020 Euros”. Economists do this all the time. Opinions vary …

“Euro as of 2020”, which is a quite arbitrary measure (especially when 2020 is not even within the reporting periode)

Note the “say”, indicating that it is just an example:

“Euro as of 2020”, say

Examples are arbitrary by their very nature.

  1. People might mix up “EUR” and “EU0” in their transactions.

They might also mix up “EUR” and “EGP” (but they usually don’t). It’s not necessary to even include virtual currencies in the menus for transactions.

  1. Most imporantly: This approach is much less flexible. There is currently (as far as I know) no way to add your own currencies to PP, so one would be restricted to a set of built-in inflation indices. However, I would be very much interested in calculating returns adjusted for money market interested rates (i.e. using indices like EONIA or X-month EURIBOR instead of an inflation index) and that seems to be impossible with the currency-based approach.

Why are you fixated on the current situation? As you yourself say, anything can be changed. And creating the option to add private currencies is certainly a more worthy use of time than to create redundant and broken widgets.

However, your concern about redundancy is very valid. But I think this can be adressed by building the adjustment for an index within the PerformanceIndex class (and not within a GUI class like the IRR widget).

Then you have the redundancy (another thing exactly like the currency conversion) in PerformanceIndex instead of a GUI class. Some improvement, but not much.


double irr = getDashboardData().calculate(get(DataSeriesConfig.class).getDataSeries(), reportingPeriod)
.getPerformanceIRR();
return (irrBench != -1) ? (1 + irr) / (1 + irrBench) - 1 : irr - irrBench;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the business logic of the widget, and it’s wrong. Since the IRR is money-weighted, it is not enough to just subtract the benchmark; you need to correct the whole data series by the benchmark before calculating the IRR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants