-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Add option to execute only the last transform in TransformWrapper and have WordBagEstimator return transformer chain #3700
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3700 +/- ##
==========================================
+ Coverage 72.77% 72.9% +0.12%
==========================================
Files 808 809 +1
Lines 145588 145599 +11
Branches 16250 16243 -7
==========================================
+ Hits 105956 106144 +188
+ Misses 35207 35035 -172
+ Partials 4425 4420 -5
|
if you can add some models to https://github.com/dotnet/machinelearning/tree/master/test/data/backcompat like ep_modelX.zip and update respectful test that would be nice thing to have. #Resolved |
src/Microsoft.ML.Data/DataLoadSave/LegacyCompositeDataLoader.cs
Outdated
Show resolved
Hide resolved
There are two estimators in our code base that use Refers to: src/Microsoft.ML.Data/DataLoadSave/TransformWrapper.cs:98 in 0879374. [](commit_id = 0879374, deletion_comment = False) |
I have added a test. In reply to: 491164237 [](ancestors = 491164237) |
Yes, done. thanks this was a great suggestion, turns out we had an existing bug in the code where if you use the above estimators more than once in the pipeline it will crash. I have changed them to return transformer chain as per your suggestion. In reply to: 491371956 [](ancestors = 491371956) Refers to: src/Microsoft.ML.Data/DataLoadSave/TransformWrapper.cs:98 in 0879374. [](commit_id = 0879374, deletion_comment = False) |
The test you added doesn't test the In reply to: 491938454 [](ancestors = 491938454,491164237) |
Sorry, Ivan didn't mention the repo actually. Here it is: https://github.com/dotnet/machinelearning-testdata. @eerhardt has permissions to push a new nuget package containing the models to myget, maybe he can help. In reply to: 491993425 [](ancestors = 491993425,491938454,491164237) |
We can check-in models in the current repo too. In reply to: 491994611 [](ancestors = 491994611,491993425,491938454,491164237) |
This and the ctor creating a Refers to: src/Microsoft.ML.Data/DataLoadSave/TransformWrapper.cs:12 in 0e3dc6a. [](commit_id = 0e3dc6a, deletion_comment = False) |
src/Microsoft.ML.Transforms/Text/WordHashBagProducingTransform.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixes #3699