Backup and restore callback #701

adi-kmt · 2023-08-11T13:49:46Z

Based on the callback issue

fchollet

Thanks for the PR!

keras_core/callbacks/backup_and_restore_callback.py

fchollet · 2023-08-11T20:01:05Z

keras_core/callbacks/backup_and_restore_callback.py

+        Get training state from temporary file and restore it
+        """
+        super().on_train_begin()
+        self.set_model(keras_core.saving.load_model(filepath=self.backup_dir))


Is this the right approach? Should we rather take self.model and load its state?

Not able to directly use self.model but self._model seems to be working

keras_core/callbacks/backup_and_restore_callback.py

sampathweb · 2023-08-31T16:15:51Z

Thanks for making the changes. I will review it today.

fchollet · 2023-08-26T01:08:41Z

keras_core/callbacks/backup_and_restore_callback.py

+
+
+@keras_core_export("keras_core.callbacks.BackupAndRestoreCallback")
+class BackupAndRestoreCallback(Callback):


It's just BackupAndRestore

fchollet · 2023-08-26T01:09:14Z

keras_core/callbacks/backup_and_restore_callback.py

+          `BackupAndRestore` callback of another training run,
+          or by another callback
+          (e.g. `ModelCheckpoint`) of the same training.
+        save_freq: `'epoch'`, integer, or `False`. When set to `'epoch'`


Use double quotes (") as the quote character

fchollet · 2023-08-26T01:09:31Z

keras_core/callbacks/backup_and_restore_callback.py

+            raise ValueError("Empty `backup_dir` argument passed")
+        self.file_path = file_path
+
+        if (not save_freq) and (not save_before_preemption):


No need for parens here

fchollet · 2023-08-26T01:09:53Z

keras_core/callbacks/backup_and_restore_callback.py

+            self._model = keras_core.saving.load_model(filepath=self.file_path)
+
+    def on_train_end(self, logs=None):
+        if self.delete_checkpoint and \


Remove \ line breaks and let black handle the code formatting

I think this is from an old file, don't see it in my code 😅

sampathweb · 2023-09-01T04:37:06Z

keras_core/callbacks/backup_and_restore_callback.py

+            self._cleanup_checkpoint()
+
+    def on_epoch_begin(self, epoch, logs=None):
+        if self.delete_checkpoint and self._check_checkpoints_exists(


Why do we delete checkpoint on Epoch begin? If an interrupt happens during training, it will not be able to recover. Deletion of checkpoint should only happen in on_train_end. I checked to confirm that this logic of deleting in on_epoch_begin is not there in tf.keras implementation.

The issue I was trying to consider is that there should be only a single checkpoint at a time according to the documentation, but I understand your point, if the saving frequency is epoch and the model get interrupted between the epoch it will not be able to recover.
So I'm thinking of adding it in here i.e. just before saving the weights.
Then also delete it at train end.

With overwrite=True, why do we need to delete before writing again? Deleting at on_train_end makes sense and would be consistent with tf.keras

After removing the delete checkpoint logic in on_epoch_begin, I am good with this PR.

sampathweb · 2023-09-01T16:16:30Z

keras_core/callbacks/backup_and_restore_callback.py

+            self._cleanup_checkpoint()
+
+    def on_epoch_begin(self, epoch, logs=None):
+        if self.delete_checkpoint and self._check_checkpoints_exists(


After removing the delete checkpoint logic in on_epoch_begin, I am good with this PR.

fchollet · 2023-09-01T17:09:52Z

keras_core/callbacks/backup_and_restore_callback.py

+
+    Args:
+        file_path: String, path to store the checkpoint.
+          e.g. `backup_dir = os.path.join(working_dir, "backup")`.


Please use 4 space indent

We should stick to the original argument, backup_dir. Then we can use it to generate the filepath internally. Note that the filepath needs to end in .weights.h5.

@kamathis4 - Can you address this comment

fchollet · 2023-09-01T17:10:16Z

keras_core/callbacks/backup_and_restore_callback.py

+    ...   def on_epoch_begin(self, epoch, logs=None):
+    ...     if epoch == 4:
+    ...       raise RuntimeError('Interrupting!')
+    >>> callback = keras.callbacks.BackupAndRestore(backup_dir="/tmp/backup")


(This is what is being shown in this code example)

fchollet · 2023-09-01T17:12:53Z

keras_core/callbacks/backup_and_restore_callback.py

+        """Returns the file path for checkpoint."""
+
+        try:
+            # `filepath` may contain placeholders such as


Since the filename will be autogenerated you can just include epoch in the filename you pick -- e.g. backup_at_epoch_{epoch}.weights.h5

sampathweb · 2023-09-01T21:11:46Z

keras_core/callbacks/backup_and_restore_callback.py

+        Get training state from temporary file and restore it
+        """
+        if self._check_checkpoints_exists(self.file_path):
+            self._model.load_weights(filepath=self.file_path)


We are not restoring the "Current Epoch Number", so the model.fit that happens after the restore re-trains for all the epochs. In tf.keras implementation, it restores the current epoch and model.fit only trains for the remaining epochs. See example in https://keras.io/api/callbacks/backup_and_restore/

Marking it resolved here. We will address it in subsequent PR when we add the hook in model to set initial_epoch.

sampathweb · 2023-09-01T21:13:26Z

keras_core/callbacks/backup_and_restore_callback_test.py

+                x_train, y_train, batch_size=4, callbacks=[cbk], epochs=5
+            )
+
+            self.assertEqual(cbk._current_epoch, 4)


Can you also assert that len(hist["loss"]) == 3 to confirm that it only ran for 3 additional epochs and not the entire 5 epochs again? I would expect that assertion to fail since we don't seem to restore the epoch in on_train_begin.

@fchollet - What do you think we should do? Is there a good way we can set the iniital_epoch of the model? Also, we are not restoring the Optimizer state that tf.keras had restored when it saved it in training checkpoint. Any suggestions.

Checked with Francois and addressed both questions -

Model saving also includes optimizer, so optimizer state is also restored when backup is restored.

On setting initial_epoch, we may need to add that hook on model before we can set it in the on_train_begin of BackupAndRestore. We will address that in a subsequent PR.
We can make this conversation resolved.

fchollet · 2023-09-20T18:26:47Z

Hi @kamathis4 -- are you still working on this PR? Should we take it over?

adi-kmt · 2023-09-21T03:30:14Z

Hi @kamathis4 -- are you still working on this PR? Should we take it over?

Hey @fchollet, please do take over. Have been really busy with work and don't seem to be able to make time for oss.

fchollet · 2023-09-21T04:24:06Z

Ok -- merging and patching up on top. Thanks for the contribution!

adi-kmt added 2 commits August 9, 2023 11:34

init

2a7ae9f

backup_and_restore_callback impl

b3ff9cb

adi-kmt marked this pull request as draft August 11, 2023 13:50

basic tests written

88f5aa1

fchollet reviewed Aug 11, 2023

View reviewed changes

Comments fixed, tests written

70abdc7

adi-kmt marked this pull request as ready for review August 23, 2023 13:44

adi-kmt requested a review from fchollet August 23, 2023 13:46

adi-kmt changed the title ~~[WIP}Backup and restore callback~~ Backup and restore callback Aug 23, 2023

sampathweb self-requested a review August 23, 2023 20:52

sampathweb suggested changes Aug 28, 2023

View reviewed changes

Comments fixed

eff01e8

adi-kmt requested a review from sampathweb August 29, 2023 16:57

fchollet reviewed Aug 31, 2023

View reviewed changes

fixed pr comments

5d7e30f

adi-kmt requested a review from fchollet September 1, 2023 03:14

sampathweb suggested changes Sep 1, 2023

View reviewed changes

adi-kmt requested a review from sampathweb September 1, 2023 06:47

sampathweb suggested changes Sep 1, 2023

View reviewed changes

Removed on epoch begin checkpoint clean

8f281d2

adi-kmt requested a review from sampathweb September 1, 2023 16:41

fchollet reviewed Sep 1, 2023

View reviewed changes

sampathweb suggested changes Sep 1, 2023

View reviewed changes

adi-kmt requested a review from fchollet September 21, 2023 03:32

fchollet merged commit 5c11fe6 into keras-team:main Sep 21, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Backup and restore callback #701

Backup and restore callback #701

adi-kmt commented Aug 11, 2023 •

edited

Loading

fchollet left a comment

fchollet Aug 11, 2023

adi-kmt Aug 23, 2023

sampathweb commented Aug 31, 2023

fchollet Aug 26, 2023

fchollet Aug 26, 2023

fchollet Aug 26, 2023

fchollet Aug 26, 2023

adi-kmt Sep 1, 2023

sampathweb Sep 1, 2023

adi-kmt Sep 1, 2023

sampathweb Sep 1, 2023 •

edited

Loading

sampathweb Sep 1, 2023

adi-kmt Sep 1, 2023

sampathweb Sep 1, 2023

fchollet Sep 1, 2023

sampathweb Sep 8, 2023

fchollet Sep 1, 2023

fchollet Sep 1, 2023

sampathweb Sep 1, 2023

sampathweb Sep 8, 2023

sampathweb Sep 1, 2023

sampathweb Sep 1, 2023

sampathweb Sep 8, 2023

fchollet commented Sep 20, 2023

adi-kmt commented Sep 21, 2023

fchollet commented Sep 21, 2023



		@keras_core_export("keras_core.callbacks.BackupAndRestoreCallback")
		class BackupAndRestoreCallback(Callback):

Backup and restore callback #701

Backup and restore callback #701

Conversation

adi-kmt commented Aug 11, 2023 • edited Loading

fchollet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sampathweb commented Aug 31, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sampathweb Sep 1, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fchollet commented Sep 20, 2023

adi-kmt commented Sep 21, 2023

fchollet commented Sep 21, 2023

adi-kmt commented Aug 11, 2023 •

edited

Loading

sampathweb Sep 1, 2023 •

edited

Loading