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

cvxsimulator in requirements #67

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

Conversation

tschm
Copy link
Collaborator

@tschm tschm commented Jan 10, 2024

No description provided.

@tschm tschm linked an issue Jan 10, 2024 that may be closed by this pull request
@tschm
Copy link
Collaborator Author

tschm commented Jan 10, 2024

Maybe for later. Would help to focus the code better on the actual Markowitz experiments. @kasperjo also has done good work with the simulator. Introduces a bunch of dependencies. Note that I have a copy of quantstats in a folder of cvxsimulator to avoid dealing with this dependency with only sporadic updates...

@tschm tschm requested a review from phschiele January 10, 2024 04:21
@phschiele
Copy link
Collaborator

phschiele commented Jan 10, 2024

I'm not in favour, this was supposed to be an encapsulated implementation of the paper with rather minimal dependencies.
Fully in favour of making use of it in cvxmarkowitz, of course!

@tschm
Copy link
Collaborator Author

tschm commented Jan 17, 2024

Recent versions of cvxSimulator only depend on plotly. I have made the dependency on quantstats an optional extra. In case you are interested in performance metrics you can install that, too. However, a lot of users would have something for their metrics in place already.

@phschiele
Copy link
Collaborator

Thanks for the updates to cvxSimulator @tschm!
I still think it is not necessary to add the dependency in this repo, since we don't use it at all.
So either we rewrite the backtest using cvxsimulator, or close this PR.
I'd be in cautious rewriting the backtest, since these are the numbers that we also reported in the paper, and changes in the setup might lead to (small) deviations.

@tschm
Copy link
Collaborator Author

tschm commented Apr 6, 2024

of course, the plan would be to first include the dependency and then refactor the backtest... It could save some code and put emphasis on the core message of the paper.

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.

Include dependency on cvxSimulator
3 participants