TimeStopping callback is untested.

[gabrieldemarmiesse]

We have no way of knowing if it works or not. Some minimal tests would be greatly appreciated.

See #964

[failure-to-thrive]

Currently all the Callbacks are not covered with tests. And they should be uniform. Any ideas how it should look like?

[gabrieldemarmiesse]

We should just have a fit() call and then we check that the callback is doing what we expect it to do. For the TimeStopping callback, we can check the running time of the fit call for example.

[failure-to-thrive]

So here is a concept. It is so small that I put it here:

import sys

import numpy as np
from tensorflow.keras.models import Sequential
from tensorflow.keras.layers import Dense

from tensorflow_addons.callbacks import TimeStopping


X = np.array([[0,0],[0,1],[1,0],[1,1]])
y = np.array([[0],[1],[1],[0]])

model = Sequential()
model.add(Dense(2))
model.add(Dense(1))
model.compile(loss='mean_squared_error')

def test():
    try:
        time_stopping = TimeStopping(2, verbose=1) # Stop after 2 seconds, verbose on;
        model.fit(X, y, epochs=10000, verbose=0, callbacks=time_stopping)

        time_stopping = TimeStopping(2, verbose=0) # Stop after 2 seconds, verbose off;
        model.fit(X, y, epochs=10000, verbose=0, callbacks=time_stopping)

        time_stopping = TimeStopping(verbose=1) # Let it go till end, verbose on;
        model.fit(X, y, epochs=1000, verbose=0, callbacks=time_stopping)
    except:
        return 1
    return 0

if __name__ == "__main__":
    sys.exit(test())

If you need time checks, please point me to a module to use. However, since the execution time is a non-determinant entity and we do not rely on any external watchdog, the test might become flaky.

[gabrieldemarmiesse]

Thanks a lot for this proof of concept :)

However, since the execution time is a non-determinant entity and we do not rely on any external watchdog, the test might become flaky.

That's true. Maybe we can hack things. What about keeping your model and adding layer with dynamic=True to force eager execution and put a time.sleep(1) inside the call? We can then measure the time for let's say 2 epochs and ensure 2<= t <=3. What do you think?

@shun-lin who may have ideas.

[failure-to-thrive]

That's true. Maybe we can hack things. What about keeping your model and adding layer with dynamic=True to force eager execution and put a time.sleep(1) inside the call? We can then measure the time for let's say 2 epochs and ensure 2<= t <=3. What do you think?

Sounds feasible but looks too complicated. 😄 What if to stay with a simple timing and see how it goes? Chances are, we are unreasonable pessimistic on non-determinant. 😄

[gabrieldemarmiesse]

I'll try to do a PR based on your code with a wait layer and I'll let you review. If that's too complicated we'll drop the idea.

[failure-to-thrive]

OK. But please keep in mind that there is a program code flow outside of layers. Also, what if the OS thread scheduler suspends the test's thread? Timing in a multitasking environment is a tricky job...

[gabrieldemarmiesse]

Thanks for all the pointers! That's definitly tricky.