Added the assembly name of the custom transform to the model file

[harishsk]

Fixes #4965
Right now when a custom transformer is used, the model file retains the name of the transformer as a string. This is not enough information to re-instantiate the custom transformer when the model is loaded from file. If the assembly containing the custom transformer is not already registered with the component catalog, then the model will fail to load.
To fix this, I have incremented the model version and am now saving the assembly name of the transform with the model.

[antoniovs1029]

It might be good to have this test both for the model before saving it, and for the model that is loaded from disk. Ideally both models should behave the same, but if a bug is introduced that only appears in one of both cases, it might be good to have both of the tests.

The InlineDataAttribute could become handy for this to avoid code duplication, as used by the PermutationFeatureImportanceTests:

machinelearning/test/Microsoft.ML.Tests/PermutationFeatureImportanceTests.cs

Lines 30 to 51 in cacc72f

	[Theory]
	[InlineData(true)]
	[InlineData(false)]
	public void TestPfiRegressionOnDenseFeatures(bool saveModel)
	{
	var data = GetDenseDataset();
	var model = ML.Regression.Trainers.OnlineGradientDescent().Fit(data);
	ImmutableArray<RegressionMetricsStatistics> pfi;
	if(saveModel)
	{
	var modelAndSchemaPath = GetOutputPath("TestPfiRegressionOnDenseFeatures.zip");
	ML.Model.Save(model, data.Schema, modelAndSchemaPath);
	var loadedModel = ML.Model.Load(modelAndSchemaPath, out var schema);
	var castedModel = loadedModel as RegressionPredictionTransformer<LinearRegressionModelParameters>;
	pfi = ML.Regression.PermutationFeatureImportance(castedModel, data);
	}
	else
	{
	pfi = ML.Regression.PermutationFeatureImportance(model, data);
	}

#Resolved

[harishsk]

Fixed.

---

In reply to: 401428872 [](ancestors = 401428872)

[antoniovs1029]

I wonder, what happens (and what should happen) if the user actually registers the assembly as done here and then tries to load the model? Will it throw an exception, or will it work anyway with the fix on this PR? #Resolved

[harishsk]

It will work. The assembly can be registered multiple times. #Resolved

[antoniovs1029]

Great.

If anything, I would recommend using the InlineData trick I mentioned in another comment to test both cases: when the user registers the assembly manually (even if it's not necessary) and when they don't register it. #Resolved

[harishsk]

Done in the next iteration.

---

In reply to: 401794399 [](ancestors = 401794399)

[antoniovs1029]

Is there any case where the loaded model would actually require having a different name registered from the "FullName" retrieved from here? #Resolved

[antoniovs1029]

Or any case where trying to access that member of _mapAction would throw? #Resolved

[harishsk]

The call to Method can throw a MemberAccessException. But that would be up to the caller to fix in their code and the exception would help with that.

---

In reply to: 401790783 [](ancestors = 401790783)

[harishsk]

I think we should enforce the idea that the same transform that is used in training should be used in prediction as well. If they were to be different, then they are not the same pipelines and not the same models.

---

In reply to: 401431899 [](ancestors = 401431899)

[antoniovs1029]

In general, it LGTM, I just left some comments and also it would be good to update the docs and samples in here to say that it's no longer necessary for the user to register the assembly when loading back the model:

https://docs.microsoft.com/en-us/dotnet/api/microsoft.ml.custommappingcatalog.custommapping?view=ml-dotnet

[antoniovs1029]

Just curious, why do we need System.Diagnostics.Contracts in here? #Resolved

[harishsk]

Fixed.

---

In reply to: 401444214 [](ancestors = 401444214)

[antoniovs1029]

LGTM It only needs to update the docs and the samples to show that it is no longer necessary to register the assembly