capreolus-ir / capreolus

A toolkit for end-to-end neural ad hoc retrieval
https://capreolus.ai
Apache License 2.0
95 stars 32 forks source link

Validate that niters >= validatefreq #139

Closed andrewyates closed 3 years ago

ali-abz commented 3 years ago

Hi Andrew, If you are OK with this, I would like to help a little bit by fixing this issue. Do I have your permission?

andrewyates commented 3 years ago

Hi Ali, yes, that would be great! I'd be happy to discuss potential fixes as well, if it's helpful. I can think of a couple possible approaches:

  1. Raise an exception when validatefreq < niters to force the user to change their config
  2. When validatefreq < niters , reduce validatefreq to niters and log a message. The downside of this is that users might get strange behavior if they later increase niters (e.g., starting with niters=1; validatefreq=4 and then later setting niters=10), because they could miss the message and expect validation to be happening much more often that it does.
  3. Always run validation on the last epoch regardless of validatefreq.
andrewyates commented 3 years ago

Regardless of the above, it would be nice to also log a message describing how often validation is going to be performed. That would make the behavior more clear and hopefully reduce confusion.

ali-abz commented 3 years ago

As a user, I prefer option 1 since that is very explicit and there is no implicit actions behind it. I believe the user should be informed that there is problem with the config he/she provided. I would add a sanity check for validatefreq < niters if you are agreed. Also, describing the validation schedule is a great idea. Would something like "Validation is scheduled on iterations: [x, y, ...]" be good? I think its logger should be just before the main loop in train: https://github.com/capreolus-ir/capreolus/blob/7b7dc1d27bb532df29930eaa1d503886eed65a0c/capreolus/trainer/pytorch.py#L229

andrewyates commented 3 years ago

Yes, both of these changes sound good to me. Thanks for your help!