Skip to content

AveragedPerceptron default iterations #5568

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

Closed
justinormont opened this issue Dec 26, 2020 · 1 comment · Fixed by #5586
Closed

AveragedPerceptron default iterations #5568

justinormont opened this issue Dec 26, 2020 · 1 comment · Fixed by #5586
Assignees
Labels
API Issues pertaining the friendly API bug Something isn't working classification Bugs related classification tasks good first issue Good for newcomers P2 Priority of the issue for triage purpose: Needs to be fixed at some point. up-for-grabs A good issue to fix if you are trying to contribute to the project

Comments

@justinormont
Copy link
Contributor

justinormont commented Dec 26, 2020

When changing AveragedPerceptron's default number of iterations from 1 to 10 in #5258, we missed one.

The main interface continues to default to 1 iteration:
image

Previous issues:

Code location

Default is set here:

int numberOfIterations = Options.AveragedDefault.NumberOfIterations)

Which originates from:

internal class AveragedDefault : OnlineLinearOptions.OnlineDefault
{
public const float LearningRate = 1;
public const bool DecreaseLearningRate = false;
public const float L2Regularization = 0;
}

Which in-turn inherits its value from:

internal class OnlineDefault
{
public const int NumberOfIterations = 1;
}

Possible fix

We may want to make a new class within AveragedPerceptron to hold its overrides:

        internal class AveragedPerceptronDefault : AveragedDefault
        { 
            public const float NumberOfIterations = 10; 
        } 

/cc @michaelgsharp

@justinormont justinormont added bug Something isn't working good first issue Good for newcomers API Issues pertaining the friendly API up-for-grabs A good issue to fix if you are trying to contribute to the project classification Bugs related classification tasks labels Dec 26, 2020
@antoniovs1029 antoniovs1029 added the P2 Priority of the issue for triage purpose: Needs to be fixed at some point. label Dec 28, 2020
@michaelgsharp
Copy link
Contributor

I think your idea of making a new class for just the AveragedPerceptron overrides is a good idea. I'll get to this soon.

@michaelgsharp michaelgsharp self-assigned this Jan 11, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Mar 17, 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 bug Something isn't working classification Bugs related classification tasks good first issue Good for newcomers P2 Priority of the issue for triage purpose: Needs to be fixed at some point. up-for-grabs A good issue to fix if you are trying to contribute to the project
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants