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

fix some minor bugs in source code/examples/tests #5

Closed
walkacross opened this issue Feb 18, 2018 · 6 comments
Closed

fix some minor bugs in source code/examples/tests #5

walkacross opened this issue Feb 18, 2018 · 6 comments

Comments

@walkacross
Copy link

hello @enzobusseti @SteveDiamond @jonathanng

thank you for providing this great framework for portfolio optimization. I am wondering would you mind spent a little time fixing some minor bugs in source code and in examples/tests.

Some bugs likes:
(1) Can't instantiate abstract class MPOReturnsForecast with abstract methods weight_expr.

(2) ValueError: Incompatible dimensions (430, 1) (431, 1)

(3) parameter name mistake, like returns_forecast and alpha_model.

(4) the first argument of SPO policy in SPO examples.

thanks for your time and have a good day.

@sunilshah
Copy link

@jonathanng

Fixes in your current fork (jonathanng/cvxportfolio) seem reasonable, yet the examples do not work:

(1) Need to fix keyword change in call to MarketSimulator in HelloWorld and SinglePeriodOptimization

(2) After the name change the, HelloWorld gives reasonable results, but SinglePeriodOptimization gives negative returns for most of the coarse search parameters. The results do not correspond to plots in the notebook in the repository ( or in the paper).

Thanks.

@jonathanng
Copy link
Contributor

Try the examples in my branch. Those should work.

@sunilshah
Copy link

Hi @jonathanng,

Thanks for getting back.

I tried your master branch (https://github.com/jonathanng/cvxportfolio, latest commit). My earlier comments are about the examples and the code in that branch (using the latest available versions of python 3.6 and notebook on Homebrew).

HelloWorld errors out in call to MarketSimulator:


TypeError Traceback (most recent call last)
in ()
2 market_returns = returns,
3 costs = [tcost_model, hcost_model],
----> 4 cash_key = 'USDOLLAR'
5 )
6

TypeError: init() got an unexpected keyword argument 'market_returns'

Upon correcting the keyword to your March 2 commit, the notebook runs and gives reasonable results.

PortfolioSimulation.ipynb also needs the same correction. However, it generates results that are very different ( Active Risk at 19.3%) from those in the notebook on github.

SinglePeriodOptimization also generates very different results after the keyword correction. Most of the results of coarse search have very large negative excess returns.

Please re-run the examples. If you post the corrections in your branch I will be happy to test them out.

@ghost
Copy link

ghost commented Apr 23, 2018

I did a bug-fixing pass, I believe everything in the examples works right now.

@ghost ghost closed this as completed Apr 23, 2018
@jpiter
Copy link

jpiter commented Apr 4, 2019

Can you please let me know where I can find fixed examples. It's been a year, so I assume the master branch must have been updated, but I still see the same error on MPOReturnsForecast with abstract methods weight_expr. TIA.

@esvhd
Copy link

esvhd commented Jul 7, 2020

I did a bug-fixing pass, I believe everything in the examples works right now.

Hi @enzobusseti,

I'm still running into problems trying to instiantiate MPOReturnsForecast class as well after cloning and installing the latest master branch here. Is the fix here simply to create a dummy weight_expr() method for the class since it's not being used?

---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-24-d1b92aa49092> in <module>
----> 1 returns_forecast = cp.MPOReturnsForecast(all_return_estimates)
      2 results_MPO={}

TypeError: Can't instantiate abstract class MPOReturnsForecast with abstract methods weight_expr

This issue was closed.
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

No branches or pull requests

5 participants