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

FastTree EarlyStoppingRule definition is inconsistent with API #2520

Closed
rogancarr opened this issue Feb 13, 2019 · 2 comments
Closed

FastTree EarlyStoppingRule definition is inconsistent with API #2520

rogancarr opened this issue Feb 13, 2019 · 2 comments
Assignees
Labels
API Issues pertaining the friendly API
Projects

Comments

@rogancarr
Copy link
Contributor

In FastTree, the EarlyStoppingRule is defined as

[Argument(ArgumentType.Multiple, HelpText = "Early stopping rule. (Validation set (/valid) is required.)", ShortName = "esr", NullName = "<Disable>")]
[TGUI(Label = "Early Stopping Rule", Description = "Early stopping rule. (Validation set (/valid) is required.)")]
public IEarlyStoppingCriterionFactory EarlyStoppingRule;

This can be specified like so:

var fastTreeTrainer = mlContext.Regression.Trainers.FastTree(new 
    Trainers.FastTree.FastTreeRegressionTrainer.Options {
        EarlyStoppingMetrics = 2,
        EarlyStoppingRule = new GLEarlyStoppingCriterion.Arguments()
    });

This exposes the IComponentFactory way of doing things in the public API, which seems to be inconsistent with the public API. One suggestion is to use an enum over the existing options.

That said, this does support custom early stopping methods through implementing IEarlyStoppingCriterionFactory.

@rogancarr rogancarr added this to To do in Project 13 via automation Feb 13, 2019
@rogancarr rogancarr added the API Issues pertaining the friendly API label Feb 13, 2019
@wschin wschin self-assigned this Mar 1, 2019
@wschin wschin moved this from To do to In progress in Project 13 Mar 1, 2019
@wschin
Copy link
Member

wschin commented Mar 1, 2019

It will be a part of PR #2753 which will fix #2617.

@wschin
Copy link
Member

wschin commented Mar 4, 2019

We can't make them enum because they all have their own options such as WindowSize below.

    public sealed class UPEarlyStoppingCriterion : EarlyStoppingCriterion<UPEarlyStoppingCriterion.Options>
    {
        public sealed class Options : OptionsBase, IEarlyStoppingCriterionFactory
        {
            public int WindowSize = 5;
        }

Project 13 automation moved this from In progress to Done Mar 6, 2019
@ghost ghost locked as resolved and limited conversation to collaborators Mar 24, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
API Issues pertaining the friendly API
Projects
No open projects
Project 13
  
Done
Development

No branches or pull requests

2 participants