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

The function could black_litterman.market_implied_prior_return should be revised #574

Open
siliconMagic opened this issue Jan 7, 2024 · 0 comments
Labels
bug Something isn't working

Comments

@siliconMagic
Copy link

siliconMagic commented Jan 7, 2024

Describe the bug
The parameter cov_matrix required to be pd.DataFrame, but matrix multiplier may be error due to the issue of matrix align. But if we pass the values of matrix to cov_matrix, the warnings show.

Expected behavior
No warnings, no errors.

Code sample
Just a little change could be useful

 if not isinstance(cov_matrix, pd.DataFrame):
        warnings.warn(
            "If cov_matrix is not a dataframe, market cap index must be aligned to cov_matrix",
            RuntimeWarning,
        )
    mcaps = pd.Series(market_caps)
    mkt_weights = mcaps / mcaps.sum()
    # Pi is excess returns so must add risk_free_rate to get return.
    return risk_aversion * cov_matrix.values.dot(mkt_weights) + risk_free_rate # change here!

Also, the parameter risk_free_rate should be used with frequency. For example, if we use weekly data, then the extra parameter such as freq=52 should be provided by users, and the code should be added:

risk_free_rate = risk_free_rate / freq

Operating system, python version, PyPortfolioOpt version
Win 11, python 3.8.15, PyPortfolioOpt 1.2.6

@siliconMagic siliconMagic added the bug Something isn't working label Jan 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant