Marigold / universal-portfolios

Collection of algorithms for online portfolio selection
Other
779 stars 214 forks source link

Minor fixes #40

Closed alexander-myltsev closed 3 years ago

Marigold commented 3 years ago

Looks good! Left just one minor comment.

On a side note, if you're experimenting with Kelly portfolio you should try out MPT. It's a generalization of Kelly, is more robust and can be better controlled (e.g. long only etc.).

alexander-myltsev commented 3 years ago

@Marigold would you merge it?

Marigold commented 3 years ago

@alexander-myltsev merged, btw could you add links to relevant papers to docstrings (in some future PR when convenient)? Never heard of these algos and would love to learn more.

alexander-myltsev commented 3 years ago

@Marigold I am so sorry :) I messed up git commits. I completely missed that fixes branch is at PR. Those both Kelly algorithms implementations are my state-of-the-art implementation and under active research. They simply don't work as expected :) You should revert the PR from me and merge the current fixes branch. Or could you grant me temporary access to the repo so I can fix it myself?

Marigold commented 3 years ago

No worries, happened to me hundreds of times... Added you as a collaborator, ping me if this doesn't give you sufficient permissions.

alexander-myltsev commented 3 years ago

I need rights to push force to rewrite history. Could you add the permission?

Marigold commented 3 years ago

Enabled force push to the repo. Try it out, if that doesn't work for you I'll do it myself (couldn't find specific collaborator permissions).

alexander-myltsev commented 3 years ago

Thanks. I have almost done:

$ git push -f marigold master
Enumerating objects: 1, done.
Counting objects: 100% (1/1), done.
Writing objects: 100% (1/1), 240 bytes | 48.00 KiB/s, done.
Total 1 (delta 0), reused 0 (delta 0)
remote: error: GH006: Protected branch update failed for refs/heads/master.
remote: error: At least 1 approving review is required by reviewers with write access.
To github.com:Marigold/universal-portfolios.git
 ! [remote rejected] master -> master (protected branch hook declined)
error: failed to push some refs to 'git@github.com:Marigold/universal-portfolios.git'

I suggest you turn off those policies until I force push to master.

alexander-myltsev commented 3 years ago

@Marigold or just force master branch to marigold-master. And make push -f on your side.

Marigold commented 3 years ago

Forced pushed myself. Sorry, I should have done it right away. Thanks for help on this.

alexander-myltsev commented 3 years ago

Thank you! Again, sorry for the mess.