emanjavacas / pie

A fully-fledge PyTorch package for Morphological Analysis, tailored to morphologically rich and historical languages.
MIT License
22 stars 10 forks source link

Accuracy function #54

Open PonteIneptique opened 4 years ago

PonteIneptique commented 4 years ago

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.

PonteIneptique commented 4 years ago

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 commented 4 years ago

Did you find out why?

PonteIneptique commented 4 years ago

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

PonteIneptique commented 4 years ago

This is good to go.

PonteIneptique commented 4 years ago

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

https://github.com/emanjavacas/pie/blob/230e0e69117f3800e16dd3d8d3eb37ae08d59e7a/pie/trainer.py#L202-L204

https://github.com/emanjavacas/pie/blob/230e0e69117f3800e16dd3d8d3eb37ae08d59e7a/pie/trainer.py#L154-L155

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

emanjavacas commented 4 years ago

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 commented 4 years ago

I don't know, probably had a misunderstanding or did not check correctly. There is basically no reason it should affect lr schedule...

As for the second comment, yes, maybe we should have a lr_threshold?

Le mar. 4 août 2020 à 5:10 PM, Enrique Manjavacas notifications@github.com a écrit :

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...

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/emanjavacas/pie/pull/54#issuecomment-668654524, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAOXEZXJWKZQUM6OJHAZBBDR7AQHZANCNFSM4LEMOL6Q .

PonteIneptique commented 4 years ago

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