added --ignore_existing_predictions flag to run_model.py to overwrite existing predictions#48
Conversation
| # are reprocessed from scratch. | ||
| if local_file_exists(_PREDICTIONS_JSON.value): | ||
| print(f"Deleting existing predictions in `{_PREDICTIONS_JSON.value}`.") | ||
| Path(_PREDICTIONS_JSON.value).unlink() |
There was a problem hiding this comment.
Can we make a backup instead of deleting the file? I can see myself making this dangerous mistake and lose previous work.
|
Re: backup... I think you got this, but for anyone reading this in the future, clarifying that the default is still not to overwrite, you have to add this option explicitly to delete existing predictions. For my use cases, I really want the predictions to go away forever, this is primarily about minimizing confusion. So making a backup makes the problem marginally worse for me. Another option is to change this from a boolean parameter to something like "--overwrite_behavior", which could be:
This approach is fine with me, I'm 50/50 on this vs. what I did in this PR (default resume, explicit flag to overwrite). LMK whether you prefer the --overwrite_behavior approach I'm suggesting here? If so, I'll change that (and ask for an updated review) before merging this PR. |
|
I'm fine to leave it as is. I might never use that flag, and I know it won't affect the default behaviour. |
added --ignore_existing_predictions flag to run_model.py to overwrite existing predictions