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

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

Closed
ericstj opened this issue Jan 9, 2024 · 3 comments · Fixed by #6951
Assignees
Milestone

Comments

@ericstj
Copy link
Member

ericstj commented Jan 9, 2024

System Information (please complete the following information):

  • OS & Version: Any
  • ML.NET Version: 3.0
  • .NET Version: Any

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.
@ghost ghost added the untriaged New issue has not been triaged label Jan 9, 2024
@ericstj
Copy link
Member Author

ericstj commented Jan 9, 2024

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
Copy link
Member

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
Copy link
Member

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.

LittleLittleCloud added a commit to LittleLittleCloud/machinelearning that referenced this issue Jan 10, 2024
@LittleLittleCloud LittleLittleCloud mentioned this issue Jan 10, 2024
4 tasks
@ghost ghost added the in-pr label Jan 10, 2024
@michaelgsharp michaelgsharp added this to the ML.NET 4.0 milestone Jan 10, 2024
@ghost ghost removed the untriaged New issue has not been triaged label Jan 10, 2024
michaelgsharp pushed a commit that referenced this issue Jan 10, 2024
* fix #6949

* add MoveForwardToAttribute to assembly.cs and fix search space sub namespace
@ghost ghost removed the in-pr label Jan 10, 2024
github-actions bot pushed a commit that referenced this issue Jan 10, 2024
michaelgsharp pushed a commit that referenced this issue Jan 13, 2024
* fix #6949

* add MoveForwardToAttribute to assembly.cs and fix search space sub namespace

---------

Co-authored-by: XiaoYun Zhang <xiaoyuz@microsoft.com>
@github-actions github-actions bot locked and limited conversation to collaborators Feb 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants