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

make metric configurable that triggers checkpoint writing #4

Merged
merged 1 commit into from Nov 27, 2018

Conversation

juliakreutzer
Copy link
Collaborator

Introducing a third configurable metric:

  1. eval_metric: validation metrics like BLEU or ChrF; measured on the validation set only
  2. schedule_metric: decides when the scheduler decreases the learning rate or stops learning; might be loss or ppl, might not match eval_metric (since often smoother curve)
  3. new: ckpt_metric: decides when the model parameters are saved, i.e., a checkpoint is written; default is eval_metric

Before, checkpoint saving was coupled to eval_metric (new default), but potentially not matching schedule_metric (e.g., validating models with BLEU and storing when highest reached, but adapting learning rate according to loss). Now, one can decide whether all three align or are individually set.

@bastings
Copy link
Collaborator

 # if we schedule after BLEU/chrf, we want to maximize it, else minimize
        scheduler_mode = "max" if self.schedule_metric == "eval_metric" \
            else "min"

Does this decide to use min in case of loss/perplexity?
(And should it be configurable?)

@juliakreutzer
Copy link
Collaborator Author

Yes, this is still like it was done before. For now we only implement external evaluation metrics that should be maximized (BLEU, ChrF), so at the current state we don't have to make it configurable, I think.

@bastings
Copy link
Collaborator

I think to prevent bugs in the future maybe we should do the min/max switch based on the content of the setting, not based on if it is the same as eval_metric? What do you think?

@juliakreutzer
Copy link
Collaborator Author

What do you mean by content of the setting? All our possible options for "eval_metric" have to be maximized.

@bastings
Copy link
Collaborator

I mean if we allow any metric for that in the future that needs minimization, then that code will break.
I think we should set min/max as a function of BLEU/chrF/ppl/loss.

@bastings
Copy link
Collaborator

Let's leave this is a future improvement

@bastings bastings merged commit 2a023e8 into joeynmt:master Nov 27, 2018
juliakreutzer added a commit to juliakreutzer/bandit-joeynmt that referenced this pull request Jan 23, 2020
marvosyntactical pushed a commit to marvosyntactical/joeynmt that referenced this pull request Mar 24, 2020
make metric configurable that triggers checkpoint writing
juliakreutzer pushed a commit that referenced this pull request Jun 2, 2022
enable attention plots for transformers
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.

None yet

2 participants