add NoImprovementHandler

[Ishan-Kumar2]

Fixes #2314

Description:
As described in the issue, added a generalized version of EarlyStopping where the stop_function and pass_function can be user-defined.
@sdesrozis let me know if the approach is correct, I'll start working on the tests.

Check list:

• New tests are added (if a new feature is added)
• New doc strings: description and/or example code are in RST format
• Documentation is updated (if required)

[vfdev-5]

Thanks for the PR @Ishan-Kumar2 !
I left few minor comments to address.

[vfdev-5]

We may want also the logger for NoImprovementHandler ?

[vfdev-5]

If we want to save NoImprovementHandler's internal state we have to keep _state_dict_all_req_keys, IMO

[sdesrozis]

@Ishan-Kumar2 It looks very good, thanks ! Please go ahead with the tests !

[Ishan-Kumar2]

@sdesrozis @vfdev-5 I have added the tests, a lot of them are similar to the EarlyStopping tests and might not be even needed since they both use the NoImprovementHandler functions. For instance, test_state_dict_no_improvement_handler and test_state_dict both use the same function, and having both may be redundant, should I remove one in this case?

Also, the failing test seems unrelated to this PR

[sdesrozis]

@sdesrozis @vfdev-5 I have added the tests, a lot of them are similar to the EarlyStopping tests and might not be even needed since they both use the NoImprovementHandler functions. For instance, test_state_dict_no_improvement_handler and test_state_dict both use the same function, and having both may be redundant, should I remove one in this case?

Also, the failing test seems unrelated to this PR

Yes please, removing redundancy would be great.

Let's see on our side to fix unrelated issues before merging.

Thanks again and sorry for the late answer.

[Ishan-Kumar2]

@sdesrozis I was thinking about the redundancies, while it's true that they both call the same function do you think it is still worth having retaining the test_state_dict as it ensures that the EarlyStopping is inheriting it correctly and it works for its set of stop and pass functions, just as a precaution. The same goes for the others. What do you think?

[sdesrozis]

@Ishan-Kumar2 the tests are both useful especially if we consider the evolution of the library. I was thinking about having an unique and generic test code for the both. It would imply introducing an abstraction for the test result. As the tests are quite small and clear, I'm finally not sure about that. So let's keep it simple.

I will review asap. Thanks again.