Fix weights calculation in VBaseSignalExport #9258
Conversation
| var csv = "sym,wt\n"; | ||
|
|
||
| var targets = parameters.Targets.Select(target => | ||
| PortfolioTarget.Percent(algorithm, target.Symbol, target.Quantity) |
There was a problem hiding this comment.
Hey @vb-vlb!
The original implementation is expecting percentages/weights to be passed in, as documented at https://www.quantconnect.com/docs/v2/writing-algorithms/live-trading/signal-exports/vbase#01-Introduction see also example algorithm https://github.com/QuantConnect/Lean/blob/master/Algorithm.Python/VBaseSignalExportDemonstrationAlgorithm.py .
If the API is expecting percentages/weights too this means this implementation shouldn't be doing any math at all but just passing through the values?
Please take a look at SignalExport.SetTargetPortfolioFromPortfolio() too which will call Send on VBaseSignalExport passing percent's too?
There was a problem hiding this comment.
Hey @Martin-Molinero, thanks for the prompt reply.
In the VBaseSignalExportDemonstrationAlgorithm, the portfolio targets collection consists of a single record: SPY with a quantity of 0.25.
Since we only have one position with a quantity of 0.25, the weights output should consist of a single record with SPY having a weight of 1.
like this
sym,wt
SPY,1
In fact, because PortfolioTarget.Percent creates a new target for the specified percentage, we end up receiving a quantity of SPY that needs to be bought to make it 25% of the portfolio. This results in a stamped weights file like:
sym,wt
SPY,172
the documentation states:
“Under the hood, it uses PortfolioTarget.Percent to convert your absolute target quantities into portfolio weights.”
Which implies that the expected inputs are absolute target quantities, not derived quantities.
There was a problem hiding this comment.
the portfolio targets collection consists of a single record: SPY with a quantity of 0.25.
Since we only have one position with a quantity of 0.25, the weights output should consist of a single record with SPY having a weight of 1.
hm don't think this is right really, 0.25 mean 25% if the available portfolio, that shouldn't translate to 1 IMHO. The remaining 75% of the portfolio can be allocated to something else in the next minute for example
the documentation states:
The documentation could be wrong, need an update 💯, but what matters I think is what it should/expected to be by the vbase api and consumer?
There was a problem hiding this comment.
Hey @Martin-Molinero ,
The business objective is to create a snapshot of portfolio weights that best reflects, economically, what the portfolio actually holds.
The vBase API does not analyze the contents of the file; it only stamps the hash. Maybe @greg-vbase from vBase can comment on this to confirm.
I still see two issues with the current implementation of VBaseSignalExport:
-
It takes quantity and passes it through PortfolioTarget.Percent, which returns a number of units. As a result, we end up stamping units instead of weights.
-
If I understand correctly, the targets in the signal export do not always contain all positions in the portfolio. Since we want to stamp the portfolio weights, we also need to include positions that already exist in the portfolio but are not listed in the targets.
The proposed PR addresses the issues above.
There was a problem hiding this comment.
I think if the API expects weights only, we should update QC documentation and the only change here would be something like the following
If I understand correctly, the targets in the signal export do not always contain all positions in the portfolio. Since we want to stamp the portfolio weights, we also need to include positions that already exist in the portfolio but are not listed in the targets.
This is not how it currently behaves, the API expects user to know what are the current targets, or can provide the whole portfolio. We could potentially expand the API so that the algorithm as a consumer can add a target to the current portfolio, but that would be handled by the main manager as it does with the portfolio. So we keep each ISignalExportTarget implementation as simple and normalized as possible, seems out of scope of this change
2 participants
Fix weights calculation in VBaseSignalExport.
Description
In the VBaseSignalExport.BuildCsv method, prices were not taken into account when calculating weights.
This change fixes the weight calculation logic.
Related Issue
#9259
Motivation and Context
This change is required because the current implementation of weight calculation is incorrect, as it does not take into account position prices.
Requires Documentation Change
As this change fixes a bug, no documentation changes are needed.
How Has This Been Tested?
Tested using a test algorithm.
Types of changes
Checklist:
bug-<issue#>-<description>orfeature-<issue#>-<description>