enjine-com / mcos

Implementation of Monte Carlo Optimization Selection from the paper "A Robust Estimator of the Efficient Frontier"
MIT License
55 stars 18 forks source link

Fix hrp clustering #44

Closed peterwhiteenjine closed 4 years ago

moneygeek commented 4 years ago

Ok if you correct for the split, does our algo results match Marcos elsewhere?

On Fri, May 8, 2020, 4:48 AM peterwhiteenjine notifications@github.com wrote:

@peterwhiteenjine commented on this pull request.

In mcos/optimizer.py https://github.com/enjine-com/mcos/pull/44#discussion_r422098977:

     if len(sorted_indices) == 0:

raise ValueError('sorted_indices is empty')

     if len(sorted_indices) == 1:
         return np.array([1.])
  • split_indices = np.array_split(np.array(sorted_indices), 2)
  • weights = pd.Series(1, index=sorted_indices)
  • clusters = [sorted_indices]
  • while len(clusters) > 0:

If there are an odd number of securities, array_split favors the left branch, whereas this code favors the right. So splitting [1,2,3,4,5] with array_split gives [1,2,3],[4,5] where this code gives [1,2],[3,4,5]. Sorry if I missed something that compensates for that, I just wanted to make sure we were following the paper since that split has a big effect on the weights.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/enjine-com/mcos/pull/44#discussion_r422098977, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAXTIXNN363JGACYMEB7CDDRQPWRVANCNFSM4M3OZ5CA .

peterwhiteenjine commented 4 years ago

Yes, the results match other than the split. I've put back some of your old code and kept the new split as well as sorting at the end. Our results match those from Marcos' code.

moneygeek commented 4 years ago

Then it's not really a bug. Please close the PR

On Sun, May 10, 2020, 11:13 AM peterwhiteenjine notifications@github.com wrote:

Yes, the results match other than the split. I've put back some of your old code and kept the new split as well as sorting at the end. Our results match those from Marcos' code.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/enjine-com/mcos/pull/44#issuecomment-626367282, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAXTIXKFQQWTNNEZIEG6TZ3RQ3VFTANCNFSM4M3OZ5CA .

peterwhiteenjine commented 4 years ago

Ok. Will do.

Just to make sure all is clear: our code as is produces different weights than Marcos' code because of the different splits. If I change the splitting to match Marcos', we get the same results as his code.