cvxgrp / cvxportfolio

Portfolio optimization and back-testing.
https://www.cvxportfolio.com
GNU General Public License v3.0
986 stars 257 forks source link

cvxportfolio upgrade for cvxpy version 1.0.3 #11

Closed sunilshah closed 1 year ago

sunilshah commented 6 years ago

cvxportfolio code needs to be upgraded for cvxpy 1.0.3 (current default in PyPi). Current examples and notebook fail as expected. Any plans to upgrade and test?

mladendalto commented 6 years ago

I second @sunilshah 's request. For me HalloWorld.ipynb crashes at sum_entries and mul_elemwise, which do not exist in new cvxpy.

ghost commented 6 years ago

Yes, thanks for noting this. I'll move to 1.0 asap.

wec7 commented 6 years ago

This can be closed.

sunilshah commented 6 years ago

Thanks!

The upgraded version fails on two items in my tests.

1) MultiPeriodOptimization.ipynb

fails at line 353 in policies.py

      353  z = cvx.Variable(*w.size)
      354  wplus = w + z

changing it to

            z=cvx.Variable(*w.shape)

fixes it on my system (homebrew, mac os, python3, all latest versions). That makes sense, given the changes in cvxpy.

2) All tests fail when cvxportfolio is installed using

       pip3 install -U cvxporfolio
       nosetests cvxporfolio

I am using homebrew package manager to install python3.

I clone the repository and run the cloned version of the software rather than the one installed using pip3. The cloned version runs nosetests fine in the test directory.

wec7 commented 6 years ago

Perhaps submit a pull request to @enzobusseti regarding item 1?

Regarding item 2, I think tests on the pip installed lib is expected due to missing data (I may misunderstand). It was failing even before.

wec7 commented 6 years ago

@enzobusseti thanks for taking my pull request.

But there might be some potential problems with this upgrade (not captured by the tests), I use your lib for portfolio optimization in actual investment, and recently realized the solver outputs differently (single period optimization policy). From my result speaking, older version works correctly while upgraded version must have something wrong.

I haven't dig into details yet, it might be caused by cvxpy rather than cvxportfolio, just a note here first.

sunilshah commented 6 years ago

@weiyialanchen

New definition of leverage could be a reason for the difference. cvxpy 1.0.x uses OSQP as the default QP solver while cvxpy 0.4.11 used ECOS. In my tests comparing OSQP with ECOS, I have found OSQP to be more robust, though when both solvers worked, the results were identical within tolerances.

ghost commented 6 years ago

@sunilshah regarding the test failing, I found an error in the pip package (I wasn't including the test data), which is fixed now. The error only affects the test, not the rest of the library.

@weiyialanchen I changed the definition of leverage (there was a subtle mismatch with the definition in the paper). It shouldn't have big effects, but you might get slightly different numerical results if your strategy uses the leverage limit.

Solvers: OSQP is a new but very good solver, and should have noticeably faster convergence than ECOS. I'm planning to make the default TCostModel quadratic (instead of 3/2 power) so that it compiles to a quadratic program and is solved by OSQP.

sunilshah commented 6 years ago

@enzobusseti

Thanks for fixing tests.

As for changing to OSQP as a default, please wait till issue #535 in cvxpy for OSQP is resolved (or you can use the workaround of changing maximization to minimization).

wec7 commented 6 years ago

@enzobusseti @sunilshah

I didn't use a leverage limit. But after changing to ECOS I got my expected output back, thank you guys!!

optwave commented 6 years ago

@weiyialanchen

and recently realized the solver outputs differently (single period optimization policy)

It seems that the outputs of the first experiment with the coarse grid are different from the paper for both SinglePeriodOpt (worse in some cases) and MultiPeriodOpt (better in some cases). AFAICS, I use the latest versions of cxvportfolio and cvxpy. Please find below the corresponding figures.

SinglePeriodOpt for "2012-01-01"-"2016-12-31" (note that the scaling for y-axis is different from the one given in the paper where max(y-axis) was 30%): spo_riskrewardfrontier_5y

MultiPeriodOpt for "2012-01-01"-"2016-12-31": mpo2_riskrewardfrontier_5y

BTW, in MultiPeriodOptimization.ipynb I actually use 353 z = cvx.Variable(w.size) and not 353 z = cvx.Variables(*w.shape) as suggested by @sunilshah. The latter caused AttributeError: module 'cvxpy' has no attribute 'Variables'

I am not sure what the differences are due. It would make sense to benchmark the results under both versions of cvxpy and different versions of cvxportfolio and preferably by a person who understands the internals (I am not the right person in this sense).

Assuming that the results that I get from MultiPeriodOpt are correct, it appears that the revenue results for 10% risk are now ~35% and not ~23% as in Figure 7.5 of the paper, it would be nice to fully understand the origins of this non-trivial difference.

bstellato commented 6 years ago

@sunilshah looks like the cvxpy issue is now resolved https://github.com/cvxgrp/cvxpy/issues/535

sunilshah commented 6 years ago

@bstellato

Yes, it appears to be fixed.

@optwave

Thanks for pointing out my typo, which is fixed in my comment now.

@enzobusseti

It may be worth testing out OSQP as the default solver now.

ghost commented 6 years ago

@optwave , thanks for reporting. it might also be related to #14 (there was a bug in the definition of leverage limit, it makes sense if the results changed a bit).

If there is a $3/2$ transaction cost model the problem should compile to SOCP and default to ECOS. To force OSQP you can pass solver = 'OSQP' to the SPO policy object (but that will raise an exception if you use the $3/2$ tcost). (I believe OSQP is preferable, but it's up to the user.)

cvx.Variables is a typo, feel free to make a pull request..

quant5 commented 1 year ago

@enzbus can this old issue be closed? https://github.com/cvxgrp/cvxportfolio/blob/master/setup.py#L21