Skip to content

Execution model to use async and PorfolioTarget.Tag #8828

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

arthiondaena
Copy link
Contributor

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)
  • 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:

  • My code follows the code style of this project.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes. (Since its already tested here)
  • All new and existing tests passed.
  • My branch follows the naming convention bug-<issue#>-<description> or feature-<issue#>-<description>

@arthiondaena arthiondaena changed the title Changed default async to true and passed target.Tag Execution model to use async and PorfolioTarget.Tag Jun 13, 2025
@jaredbroad
Copy link
Member

jaredbroad commented Jun 13, 2025

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?

@arthiondaena
Copy link
Contributor Author

@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.

@arthiondaena
Copy link
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Execution Model Use PorfolioTarget.Tag
2 participants