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

Add realized dividends to gross performance #3857

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

dandevaud
Copy link
Contributor

Hi @dtslvr

I saw there was an issue raised with dividends lowering the performance (#3233)
I added logic to handle dividends like sell-order for performance calculation.

I would say from economical perspective this makes most sense, as when dividends are paid the stock price will lower by the dividend amount.

Moreover, I extracted the sellOrder and dividend handling in a separate method for readability

Copy link
Member

@dtslvr dtslvr left a comment

Choose a reason for hiding this comment

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

Thank you very much for your contribution, @dandevaud! 🙂

While reviewing, I was surprised that no test had to be adjusted with your PR. I have therefore extended the only dividend test (#3891). Can you please rebase here?

Ideally, the dividend performance should be listed here separately and also included in the overall performance:

portfolio-analysis

What do you think?

Dan added 2 commits October 11, 2024 09:44
- Dividends will  be handled like sell-orders performancewise
This might be one approach to solve issue ghostfolio#3233
@dandevaud dandevaud force-pushed the feature/Add-dividend-performance-to-total-performance branch from 21fe5c7 to 6f57439 Compare October 11, 2024 07:49
@dandevaud
Copy link
Contributor Author

Thanks @dtslvr

I updated the teste accordingly.

We can list the dividend separately, but I would already expect it in the "Absolute Asset Performance".
But your input had me thinking of something else, the holding detail:

image

There the "Change" and "Change with currency effect" should be change to something like "Total Return" and "Total Return with currency effect".
Change is rather related to price changes and total return is accepted as word for all returns relating to the asset (incl. sell and dividends)

Let me know what you think and if I should adapt the overal performance as well.

@dtslvr
Copy link
Member

dtslvr commented Oct 17, 2024

portfolio-analysis

We can list the dividend separately, but I would already expect it in the "Absolute Asset Performance".

The (Absolute) Asset Performance label is indeed not an ideal label. What about:

  • Capital Gains
    • %
  • Dividend Income
    • %
  • Currency Exchange Effect
    • %

  • Total Return
    • %

There the "Change" and "Change with currency effect" should be change to something like "Total Return" and "Total Return with currency effect".

What about "Total Return" / "Total Return with currency effect" and "Total Return (%)" / "Total Return with currency effect (%)" in the holding detail dialog?

@dandevaud
Copy link
Contributor Author

dandevaud commented Oct 20, 2024

Hi @dtslvr

Please excuse my late reply.

Sounds better, but instead of dividend income we should use something like Realized Profit/Loss, which includes sells as well.
But then it gets kind of complicated for the % calculation.
Do you have an idea?

Edit: Complicated in the sense of what to use as basis. It should be the same over all 3 values and this would be total investments without reducing it by the sells.

@dandevaud
Copy link
Contributor Author

Hi @dtslvr

Again please excuse my late reply, had some busy weeks.

I revisted the theoretical performance calculation, meaning I have not yet checked the code on how to implement it as I wanted your inputs first.

The idea of splitting up the absolut net asset performance good, I would nonetheless go with realized and unrealized gains/losses.
Realized include all gains/losses from sold assets and dividends and unrealized includes the performance of the yet hold assets compared to their purchase price.

I currently face a problem with the relative performance, respectively the cost basis to use.
Assuming the following scenario:
Buying 4 stocks @ 25, it raises to 40 you sell 1, it raises to 50

t1 t2 t3
Quantity 4 3 3
Price 25 40 50
PF-Value 100 120 150
Sold - 40 -

Calculating the P/Ls would lead to:

  • Unrealized 75 ( current holding * (current price - initial price) --> 3 * (50-25))
  • Realized 15 (sold holding * (sell price - initial price) --> 1 * (40-25))
  • Total 90 (sum of the above)

For the relative performance we need to have a cost basis to calculate against.
The simplest would be the total investment, as it bests reflects the total return on ones investment.

Absolut performance Cost basis relative performance
Unrealized P/L 75 100 75%
Realized P/L 15 100 15%
Total 90 100 90%

The problem is the unrealized P/L does not correctly refelct the price change of the underlying asset. Whilst the asset price doubled, the unrealized P/L only has a performance of 75%.

I checked yahoo finance how they handle it and the use different cost basis for the different P/L calculation, but do not show a total performance value.
For unrealized P/L they use the cost basis of the shares left, in the case above this would be 75 (shares left * initial price --> 3*75).

Absolut performance Cost basis relative performance
Unrealized P/L 75 75 100%
Realized P/L 15 100 15%
Total - - -

I would go with the first approach not correctly reflecting the price change, as it accounts for all performance of a portfolio.

Whats your take on this? Or would you even choose a totally different approach?

Many thanks for your feedback and kind regards

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.

2 participants