-
Notifications
You must be signed in to change notification settings - Fork 613
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
base: master
Are you sure you want to change the base?
Conversation
8cfb317
to
f655f6d
Compare
Merged the changes into the existing IRR widget and allowed removal of the benchmark. |
- Update IRR widget to add benchmark. That way the inflation adjusted IRR can be displayed by selecting a relevant consumer price index as benchmark.
f655f6d
to
3ce9954
Compare
Sorry for being so late. I have been quite busy. Hope to have more time soon. |
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 On the other hand, creating special inflation-adjusted versions for all widgets is redundant work and a maintenance nightmare. |
@chirlu I don’t think it would be a good idea to do this with a synthetic currency for the following reasons:
However, your concern about redundancy is very valid. But I think this can be adressed by building the adjustment for an index within the |
It is more intuitive, in my opinion, to have “2020 Euros”. Economists do this all the time. Opinions vary …
Note the “say”, indicating that it is just an example:
Examples are arbitrary by their very nature.
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.
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.
Then you have the redundancy (another thing exactly like the currency conversion) in |
|
||
double irr = getDashboardData().calculate(get(DataSeriesConfig.class).getDataSeries(), reportingPeriod) | ||
.getPerformanceIRR(); | ||
return (irrBench != -1) ? (1 + irr) / (1 + irrBench) - 1 : irr - irrBench; |
There was a problem hiding this comment.
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.
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.