dotnet / machinelearning

ML.NET is an open source and cross-platform machine learning framework for .NET.
https://dot.net/ml
MIT License
8.92k stars 1.86k forks source link

Microsoft.ML.StandardTrainers references Microsoft.ML.SearchSpace but it's not included in Microsoft.ML package #6949

Closed ericstj closed 5 months ago

ericstj commented 5 months ago

System Information (please complete the following information):

Describe the bug Microsoft.ML.StandardTrainers.dll references Microsoft.ML.SearchSpace.dll but the latter is not included in the Microsoft.ML package, but the Microsoft.ML.AutoML package.

I haven't noticed any side-effects from this yet since the usage of Microsoft.ML.SearchSpace is only in attributes, but it's possible that anything trying to enumerate other attributes on these types would see FileLoadExceptions.

I discussed it with @LittleLittleCloud and suggested a couple options, if we decide we need to fix this.

  1. Just include Microsoft.ML.SearchSpace.dll in Microsoft.ML. While this is possible it may add dependencies to Microsoft.ML - one in particular is System.Text.Json which may be odd next to Newtonsoft.Json which it already references. Eventually we should try to move away from Newtonsoft to STJ, but that's not planned yet.
  2. Refactor the implementation of the attributes in SearchSpace to be pure attributes that merely expose their parameters (see guidelines for inspiration) and typeforward those down into Microsoft.ML.Core without any of the options types they internally reference.
ericstj commented 5 months ago

Here's a sample that fails:

var type = typeof(Microsoft.ML.Trainers.AveragedLinearOptions);
foreach (var field in type.GetFields())
{
    Console.WriteLine(field);

    foreach (var attr in field.GetCustomAttributes(false))
    {
        Console.WriteLine(attr);
    }
}

This will throw:

Single LearningRate
Unhandled exception. System.IO.FileNotFoundException: Could not load file or assembly 'Microsoft.ML.SearchSpace, Version=1.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51'. The system cannot find the file specified.
File name: 'Microsoft.ML.SearchSpace, Version=1.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51'
   at System.ModuleHandle.ResolveType(QCallModule module, Int32 typeToken, IntPtr* typeInstArgs, Int32 typeInstCount, IntPtr* methodInstArgs, Int32 methodInstCount, ObjectHandleOnStack type)
   at System.ModuleHandle.ResolveTypeHandle(Int32 typeToken, RuntimeTypeHandle[] typeInstantiationContext, RuntimeTypeHandle[] methodInstantiationContext)
   at System.Reflection.RuntimeModule.ResolveType(Int32 metadataToken, Type[] genericTypeArguments, Type[] genericMethodArguments)
   at System.Reflection.CustomAttribute.FilterCustomAttributeRecord(MetadataToken caCtorToken, MetadataImport& scope, RuntimeModule decoratedModule, MetadataToken decoratedToken, RuntimeType attributeFilterType, Boolean mustBeInheritable, ListBuilder`1& derivedAttributes, RuntimeType& attributeType, IRuntimeMethodInfo& ctorWithParameters, Boolean& isVarArg)
   at System.Reflection.CustomAttribute.AddCustomAttributes(ListBuilder`1& attributes, RuntimeModule decoratedModule, Int32 decoratedMetadataToken, RuntimeType attributeFilterType, Boolean mustBeInheritable, ListBuilder`1 derivedAttributes)
   at System.Reflection.CustomAttribute.GetCustomAttributes(RuntimeModule decoratedModule, Int32 decoratedMetadataToken, Int32 pcaCount, RuntimeType attributeFilterType)
   at System.Reflection.CustomAttribute.GetCustomAttributes(RuntimeFieldInfo field, RuntimeType caType)
   at Program.<Main>$(String[] args) in C:\scratch\mlattributes\Program.cs:line 6

Not sure if that's a scenario - we should have a look at the places in ML.NET itself that actually examine custom attributes to see if anything would fail like this.

michaelgsharp commented 5 months ago

This ends up only causing issues with the Command Line parser, and so only hits users of MAML and the CLI, but it is a real product bug in our code base.

The spot seems to be InputBuilder.cs line 70 where the final blowup happens.

michaelgsharp commented 5 months ago

Most people won't ever end up hitting this as most people don't use the CLI, but there are users who do use it and any one who does use it will hit this error (I think 100% of the time). From my understanding, there is no workaround an end user could do to avoid this if they do hit it.