Wandb: log best and latest models

[Zatteliet]

I'm experimenting with Spacy's wandb integration. Awesome feature, but unless I'm missing something, there is only the option to log trained models every n steps. So if I run a training that ends up running for 290 steps and I have set it up so that model_log_interval = 100, wandb will log models at steps 100 and 200, but not the best or latest models as spacy saves them.

I know I could add a separate step to log the resulting models as artifacts into wandb, but this seems like something that could be part of the standard integration?

Hope this is the right place for this question.

[adrianeboyd]

Sure, if someone wanted to submit a PR, it would be to this repo: https://github.com/explosion/spacy-loggers/

[snosrap]

Thanks for bring this up @SandstoneGolem -- I was thinking the same thing.

Your PR does a good job of addressing this issue.

@adrianeboyd and @svlandeg, I wonder if a change to spacy's call to finalize_logger might be useful for this situation. It could pass a dictionary containing the paths to both model-best and model-last.

Benefits:

• This would eliminate the need the extra 2 parameters in the wandb PR.
• It would also prevent the hard-coding of "model-best" and "model-last" since those constants are defined in training/loop.py.

[snosrap]

Here's a more tangible example of my proposed approach in spacy/training/loop.py:

finalize_logger({
    "output_path_last": str(output_path / DIR_MODEL_LAST),
    "output_path_best": str(output_path / DIR_MODEL_BEST),
})

[svlandeg]

@adrianeboyd and @svlandeg, I wonder if a change to spacy's call to finalize_logger might be useful for this situation. It could pass a dictionary containing the paths to both model-best and model-last.

I wouldn't be in favor of passing these arguments to the finalize call of the logger. It just feels intuitively strange to me to add this kind of configuration this late into the workflow.

* It would also prevent the hard-coding of "model-best" and "model-last" since those constants are defined in training/loop.py.

There's no need to hard-code these - you can import the variables DIR_MODEL_BEST and DIR_MODEL_LAST from training/loop.py. But realistically, we won't be changing the values of those strings anytime in the foreseeable future.

* This would eliminate the need the extra 2 parameters in the wandb PR.

Practically speaking, the current wandb PR will be a quicker/easier path to get this feature released, instead of having to manage a dependency on a change in spaCy. So I'd suggest we go forward with explosion/spacy-loggers#4. Let me know what you think!