Fix bug in TextLoader #3011
Fix bug in TextLoader #3011
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3011 +/- ##
=========================================
Coverage ? 72.35%
=========================================
Files ? 803
Lines ? 143297
Branches ? 16154
=========================================
Hits ? 103680
Misses ? 35192
Partials ? 4425
|
* Breaking down the LightGBM options class into separate option classes for each LightGBM trainer. * Refactored the Boost Option classes * Hides the interface for the Booster Parameter Factory (IBoosterParameterFactory). Fixes dotnet#2559 Fixes dotnet#2618
…ng named signed. (dotnet#3010)
* Added samples for tree regression trainers. * PR comments * Added tab.
| @@ -1291,11 +1291,6 @@ private sealed class LoaderHolder | |||
| if (loader == null || string.IsNullOrWhiteSpace(loader.Name)) | |||
| goto LDone; | |||
|
|
|||
| // Make sure the loader binds to us. | |||
| var info = host.ComponentCatalog.GetLoadableClassInfo<SignatureDataLoader>(loader.Name); | |||
| if (info.Type != typeof(ILegacyDataLoader) || info.ArgType != typeof(Options)) | |||
There was a problem hiding this comment.
info [](start = 20, length = 4)
Could we explain this change? As far as I can tell, we're considering not finding this loader in the component factory to be no longer an error condition. This may be right, but I'd feel a bit more comfortable with this change if we can explain why?
I did read the attached issue, but it was a little vague on precisely why removing this check is a right and desirable thing to do.
There was a problem hiding this comment.
It seems that this check was there in order to make sure that the schema defined in the file has the correct syntax for creating a legacy TextLoader. The schema defined in the file should always be in the format TextLoader{col=... }, since it is generated by the TextSaver. Instead of the check that I deleted, should we check that loader.Name==TextLoader.LoaderSignature?
In reply to: 267105243 [](ancestors = 267105243)
There was a problem hiding this comment.
Since it is a text file, might be manually altered after saving it with the TextSaver?
In reply to: 267424076 [](ancestors = 267424076,267105243)
* Refactoring the Old ModelStatistics to a class just for the modelStats, like MLR needs all the time, and lr needs when the stats are not calculated. * Separting the Bias from the weights coefficients APIs.
…experimental nuget. (dotnet#2797) * Clean up of cancellation mechanism. * changes. * Make cancellation mechanism internal and accessible via experiemtnal nuget. * merge conflicts. * PR feedback and fix analyzer test. * Add test for public API in experimental nuget. * PR feedback. * fix tests. * PR feedback. * PR feedback. * PR feedback. * PR feedback. * PR feedback. * PR feedback. * PR feedback. * PR feedback.
* Adding functional tests for ONNX scenarios
…otnet#2976) * Step 1: create two multi-class linear models Step 2: Make SDCA trainers typed Finish version 0.1 Delete commented lines * Add some doc strings More document * Handle static extensions * Rename several maximum entropy models and trainers * Fix EP test Fix two tests and address a comment Add missing piece * Address comments * Improve option of MCSDCA * Address comments * Update code sample * Refactorize saving family * Rename a class following binary SDCA trainer
* Internalize some members of SchemaDefinition, and add tests * Code review comments * Fix build after rebase * Fix failing test * Fix build after rebase * Internalize Column ctor * Fix build after rebase
* adding XML to a public AP that had no documentation. * adding a traintest split sample. Small corrections to the images doc.xml.
* Update tests to run on mac ci leg * Updated OnnxRuntime version to 0.3.0 * Update OnnxTheory Attribute
* Binary FastTree/Forest samples using T4 templates. * Addressed comments.
…learning into tryparseschema
|
Closing this since other commits got included in it somehow. Will open a new one soon. |
ghost
locked as resolved and limited conversation to collaborators
Fixes #2996.