Support running benchmarks on netfx #2157
Conversation
There was a problem hiding this comment.
@Anipik I am very happy that you got it working with so few changes! Great job!
I left some comments, mostly nits.
| @@ -85,7 +85,7 @@ public void SetupScoringSpeedTests() | |||
| if (!File.Exists(_mslrWeb10k_Train)) | |||
| throw new FileNotFoundException(string.Format(Errors.DatasetNotFound, _mslrWeb10k_Train)); | |||
|
|
|||
| _modelPath_MSLR = Path.Combine(Directory.GetCurrentDirectory(), @"FastTreeRankingModel.zip"); | |||
| _modelPath_MSLR = Path.Combine(Path.GetDirectoryName(typeof(RankingTest).Assembly.Location), "FastTreeRankingModel.zip"); | |||
There was a problem hiding this comment.
otherwise on .netfx it creates this new model file in the root directory
There was a problem hiding this comment.
This confuses me. What do you mean by "the root directory"? Why would the current netfx have its current directory set to that?
also, the code is still using Directory.GetCurrentDirectory() in Test_Multiclass_WikiDetox_BigramsAndTrichar_OVAAveragedPerceptron. WHy doesn't that need to change?
There was a problem hiding this comment.
By root directory I mean C:\git\Mlnet\test\Microsoft.ML.Benchmarks
Yes, I missed that we need to change Test_Multiclass_WikiDetox_BigramsAndTrichar_OVAAveragedPerceptron
There was a problem hiding this comment.
Why would the current netfx have its current directory set to that?
I don't have nay concrete explaination for why it is setting current directory to C:\git\Mlnet\test\Microsoft.ML.Benchmarks
There was a problem hiding this comment.
@adamsitnik - do you happen to know? Is this something BenchmarkDotNet is controlling?
There was a problem hiding this comment.
@eerhardt Yes BDN sets the working directory to the folder with built dlls for .NET Core and to null for FX. The problem is that I dont remember why and I would prefer not to change it if I dont have to ;) It most probably has to deal with the fact that we run executable for FX and do dotnet name.dll for Core
|
@eerhardt is there anything else that needs to be done here ? |
| @@ -34,7 +34,7 @@ public abstract partial class BaseTestBaseline : BaseTestClass | |||
|
|
|||
| protected BaseTestBaseline(ITestOutputHelper output) : base(output) | |||
| { | |||
| #if NET462 | |||
| #if NET461 | |||
There was a problem hiding this comment.
NOTE: I just learned about VersionlessImplicitFrameworkDefine in the .NET Core SDK - dotnet/sdk#2073. It appears this can be changed to #if NETFRAMEWORK
There was a problem hiding this comment.
I will add this change in my next PR
ghost
locked as resolved and limited conversation to collaborators
Fixes #1945
This PR makes changes in order to enable developers to run ML benchmarks against .NetFramework