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

Accuracy function #54

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

PonteIneptique
Copy link
Contributor

Actually duplicates in part #53 because I forgot to go back to master...

This adds the ability to chose on which metric to fit in regard to devset: accuracy, f1, precision, recall, balanced-accuracy.

Default value should not change pas behaviours.

Now, instead of the default accuracy for schedule, one can choose precision, recall, balanced-accuracy or f1
as a metric for devset fitting.
@PonteIneptique
Copy link
Contributor Author

The LR Scheduler does not seem to update with this PR. I need to get into the details here but I got 25 steps for lemmatization with lr_patience 10 and patience 8 which definitely does not seem right.

@emanjavacas
Copy link
Owner

Did you find out why?

@PonteIneptique
Copy link
Contributor Author

I'll go back into having a look at this.

@PonteIneptique
Copy link
Contributor Author

This is good to go.

@PonteIneptique
Copy link
Contributor Author

I was a bit surprised (also) by the fact we still print threshold value of LROnPlateau but we do not set it.

pie/pie/trainer.py

Lines 202 to 204 in 230e0e6

self.lr_scheduler = LRScheduler(
self.optimizer, factor=settings.lr_factor,
patience=settings.lr_patience, min_lr=settings.min_lr)

pie/pie/trainer.py

Lines 154 to 155 in 230e0e6

self.lr_scheduler = optim.lr_scheduler.ReduceLROnPlateau(
optimizer, mode='max', threshold=threshold, **kwargs)

It feels to me we should forward the value of the target task but :)

@emanjavacas
Copy link
Owner

What was the issue?

Re: threshold. You are right. I never got to use it and was kept a 0.0. However, note that there are 2 thresholds here. One is for the LR schedule, the other is for the task schedule. Just saying because it's not very well documented...

@PonteIneptique
Copy link
Contributor Author

PonteIneptique commented Aug 4, 2020 via email

@PonteIneptique
Copy link
Contributor Author

Note: I think if you merge this Optuna will need to be changed a litte :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants