Execution model to use async and PorfolioTarget.Tag
Description
Changed the default asynchronous orders to true and passed target.Tag to the Execution Model.
// from
algorithm.MarketOrder(symbol, unorderedQuantity);
// to
algorithm.MarketOrder(symbol, unorderedQuantity, true, target.Tag);
Related Issue
Closes #8802
Motivation and Context
If the PortfolioTarget has a Tag, the orders generated by the Execution Model will have the same tag.
Requires Documentation Change
Probably need to change example here, to follow the same guidelines(async to true and passing target.Tag)
How Has This Been Tested?
I tested the changes by building the project and running all the existing tests on my local machine. The build was successful, and all current tests passed, confirming that my changes did not break existing functionality.
Types of changes
- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] Refactor (non-breaking change which improves implementation)
- [ ] Performance (non-breaking change which improves performance. Please add associated performance test and results)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] Non-functional change (xml comments/documentation/etc)
Checklist:
- [x] My code follows the code style of this project.
- [x] I have read the CONTRIBUTING document.
- [ ] I have added tests to cover my changes. (Since its already tested here)
- [x] All new and existing tests passed.
- [x] My branch follows the naming convention
bug-<issue#>-<description>orfeature-<issue#>-<description>
I believe this would break the code a few lines below: https://github.com/QuantConnect/Lean/pull/8828/files#diff-a3395e23424f532333a3ca8e909f3055cb5bcfc32d1c85833b4ac3229bef02e7R106
They wouldn't be filled yet so they'd stay open in the targets collection?
@jaredbroad Wouldn't the Clearfulfilled function check for if the order if fulfilled before clearing it?
public void ClearFulfilled(IAlgorithm algorithm)
{
foreach (var target in this)
{
var security = algorithm.Securities[target.Symbol];
var holdings = security.Holdings.Quantity;
// check to see if we're done with this target
if (Math.Abs(target.Quantity - holdings) < security.SymbolProperties.LotSize)
{
Remove(target.Symbol);
}
}
}
So this shouldn't have any problems if the MarketOrder is asynchronous, since the security.Holdings.Quantity value is not updated until the order is fulfilled.
Hey @jaredbroad, I tried running the python virtual environment test in my local docker lean/foundation image. All tests seems to be passing. This is the log file for python virtual environment test. As for the regression test, I also tried running it in my local machine but the errors I got are quite different when compared to the logs present in the Actions. This is the log file for regression test.
We should define asynchronous as a class variable so that users can control the behavior. E.g.:
public ImmediateExecutionModel(bool asynchonous = false)
{
_asynchronous = asynchonous;
}
algorithm.MarketOrder(security, quantity, _asynchronous, target.Tag);
I think that if we add _targetsCollection.ClearFulfilled(algorithm); to the top:
// Clear fulfilled async orders of the previous call
if (_asynchronous)
{
_targetsCollection.ClearFulfilled(algorithm);
}
_targetsCollection.AddRange(targets);
// ...
The async orders will have one timestep to fill out.
Thank you for the contribution @arthiondaena! We are opening a new PR at https://github.com/QuantConnect/Lean/pull/8872 using your initial commit as base, adding some improvements for thread safety in the lean engine, thanks again!