Skip to content
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

Migrate LEAN to .NET Core 3.0 #3340

Closed

Conversation

gsalaz98
Copy link
Contributor

@gsalaz98 gsalaz98 commented Jun 24, 2019

Description

Upgrades LEAN to .NET Core 3.0 (we can lower the required version to 2.0+ easily).

Disabled Views and Visual Studio plugin project to have project be compatible with .NET Core.

NOTE: Python algorithms will not work without an update to pythonnet.

Related Issue

#452

Motivation and Context

Performance gains on Mac OSX and Linux (100% on BasicTemplateAlgorithm and 50% on Tick data on Mac OSX)

Requires Documentation Change

Build process has not changed, but Linux + Mac OSX building is now more easy via the dotnet command line utility. Mono would not be required anymore.

How Has This Been Tested?

Able to run algorithms locally. I haven't ran regression tests and not all unit tests. I suspect a lot of work is still needed to have this equal to LEAN master, but it's a start

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • 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. Unable to run unit tests locally. Did run some tests on the Composer that completed successfully because I altered it heavily.
  • All new and existing tests passed.
  • My branch follows the naming convention bug-<issue#>-<description> or feature-<issue#>-<description>

@gsalaz98 gsalaz98 force-pushed the feature-452-dotnet-core branch from 5302066 to d52c657 Compare June 24, 2019 21:55
@@ -678,13 +213,20 @@
</None>
</ItemGroup>
<ItemGroup />
<Import Project="$(MSBuildToolsPath)\Microsoft.CSharp.targets" />
<ItemGroup>
<PackageReference Include="IKVM" Version="8.1.5717" />
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update IKVM

@@ -17,9 +17,9 @@
using System.Collections.Generic;
using System.Collections.Specialized;
using System.Linq;
using QuantConnect.Util;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixup

@@ -365,7 +365,8 @@ public enum DataFeedEndpoint
/// Getting datafeed from a QC-Live-Cloud
LiveTrading,
/// Database
Database
Database,
Tradier
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove tradier

return 0;
}
}
var startTime = DateTime.UtcNow;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rewrite this

@@ -270,7 +268,7 @@ public IEnumerable<T> GetExportedValues<T>()
{
_composableParts.Wait();
}
values = _compositionContainer.GetExportedValues<T>().ToList();
values = _compositionContainer.GetExports<T>().ToList();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make demo API and test

@@ -57,6 +58,9 @@ public ZipDataCacheProvider(IDataProvider dataProvider, bool isDataEphemeral = t
/// </summary>
public Stream Fetch(string key)
{
// Will throw while trying to read ZIP file if this line is not included
Encoding.RegisterProvider(CodePagesEncodingProvider.Instance);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

link to stackoverflow issue

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might also want to move this registration to a static constructor or somewhere else that ensure we only run it once per AppDomain

@@ -7,6 +7,7 @@
<s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/PredefinedNamingRules/=PrivateInstanceFields/@EntryIndexedValue">&lt;Policy Inspect="True" Prefix="_" Suffix="" Style="aaBb" /&gt;</s:String>
<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ECSharpKeepExistingMigration/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ECSharpPlaceEmbeddedOnSameLineMigration/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ECSharpUseContinuousIndentInsideBracesMigration/@EntryIndexedValue">True</s:Boolean>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove

@@ -810,10 +416,6 @@
<Project>{73fb2522-c3ed-4e47-8e3d-afad48a6b888}</Project>
<Name>QuantConnect.Indicators</Name>
</ProjectReference>
<ProjectReference Include="..\Jupyter\QuantConnect.Jupyter.csproj">
<Project>{9561d14a-467e-40ad-928e-ee9f758d7d98}</Project>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

review further

@@ -36,7 +38,7 @@ public override void Initialize()
AddSecurity(SecurityType.Equity, symbol, Resolution.Minute);
}

public override void OnTradeBar(Dictionary<string, TradeBar> data)
public void OnTradeBar(Dictionary<string, TradeBar> data)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

further review

@@ -357,7 +357,7 @@ private static bool DecompressOpraFile(FileInfo compressedRawDatafile, FileInfo
{
try
{
BZip2.Decompress(fileToDecompressAsStream, decompressedStream);
BZip2.Decompress(fileToDecompressAsStream, decompressedStream, true);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

review further

@@ -11,7 +11,7 @@ partial class LeanWinForm
/// Clean up any resources being used.
/// </summary>
/// <param name="disposing">true if managed resources should be disposed; otherwise, false.</param>
protected override void Dispose(bool disposing)
protected void Dispose(bool disposing)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Delete UserInterface project

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And visual studio plugin

Edit x64 build target to match AnyCpu build

Fix DotNetZip stuck on 1.10.1
@mchandschuh
Copy link
Contributor

This looks like it's still a work in progress, so I'll save the review until it reaches a slightly more stable state.

That being said, I think the move to .net core is a great one. Where possible we should strive to make the libraries compile against the .net standard libraries as they have the best portability and the most reusability (they can be referenced from .net framework projects as well).

@gsalaz98
Copy link
Contributor Author

Closing for now; will continue development on https://github.com/gsalaz98/Lean/tree/feature-452-dotnet-core-test

Lots of work has yet to be done. Most notably, porting unit tests to NUnit 3, testing composer changes, pythonnet support (python support in general)

@gsalaz98 gsalaz98 closed this Jul 23, 2019
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.

2 participants