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

Capital Changes Refactoring #1337

Merged
merged 5 commits into from Jul 25, 2016

Conversation

Projects
None yet
4 participants
@lianga888
Contributor

lianga888 commented Jul 20, 2016

  • Allow capital changes to be defined by a target value, in addition to the currently support input of deltas. The delta will be calculated on the portfolio value at the dt of the capital change
  • Refactor the checking, calculating and processing of capital changes to better support upstream processes

Andrew Liang added some commits Jul 11, 2016

Andrew Liang
MAINT: For capital changes, support input of delta or target value
For target changes, calculate the delta using the portfolio value
of the current minute
@@ -550,8 +548,8 @@ def as_account(self):
getattr(self, 'day_trades_remaining', float('inf'))
account.leverage = getattr(self, 'leverage',
period_stats.gross_leverage)
account.net_leverage = period_stats.net_leverage
account.net_leverage = getattr(self, 'net_leverage',

This comment has been minimized.

@ssanderson

ssanderson Jul 20, 2016

Member

When is this not set?

This comment has been minimized.

@lianga888

lianga888 Jul 21, 2016

Contributor

I'm not sure I follow, but previously we always calculated net_leverage from the positions zipline records in the position tracker. This change allows us to get a value from the broker, just like the other account attributes

@coveralls

This comment has been minimized.

coveralls commented Jul 20, 2016

Coverage Status

Coverage increased (+0.005%) to 83.554% when pulling b6b2278 on margin_changes into 71b2363 on master.

log.info('Processing capital change of delta %s at %s'
% (capital_change_amount, dt))
else:
log.error("Capital change %s does not indicate a valid type "

This comment has been minimized.

@jdricklefs

jdricklefs Jul 21, 2016

Member

What's your thinking on ignoring errors here? This seems pretty serious to me.

Or alternatively, what do you think of moving this validation of the given capital changes earlier in the algorithm's lifecycle so the error condition is recognized before any simulation processing?

This comment has been minimized.

@lianga888

lianga888 Jul 22, 2016

Contributor

There doesn't seem to be much validation for similar inputs done in zipline itself. In this case, upstream processes do any necessary validation before the simulation

@lianga888 lianga888 force-pushed the margin_changes branch from b6b2278 to 941ec8b Jul 25, 2016

@coveralls

This comment has been minimized.

coveralls commented Jul 25, 2016

Coverage Status

Coverage increased (+0.005%) to 83.557% when pulling 941ec8b on margin_changes into a534255 on master.

Andrew Liang added some commits Jul 12, 2016

Andrew Liang
MAINT: Refactor checking, calculation and processing of capital changes
AlgorithmSimulator will no longer check for capital changes.
Instead, TradingAlgorithm find and calculate the changes, and
PerformanceTracker will apply the changes

@lianga888 lianga888 force-pushed the margin_changes branch from 941ec8b to 0955515 Jul 25, 2016

@coveralls

This comment has been minimized.

coveralls commented Jul 25, 2016

Coverage Status

Coverage increased (+0.005%) to 83.557% when pulling 0955515 on margin_changes into a534255 on master.

@lianga888 lianga888 merged commit 2fe94d0 into master Jul 25, 2016

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@lianga888 lianga888 deleted the margin_changes branch Jul 25, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment