Repository: pytorch/ignite

Issue:
Checkpoint filename not using absolute score value
The [Checkpoint documentation](https://pytorch.org/ignite/handlers.html?highlight=checkpoint#ignite.handlers.Checkpoint) mentions the use of the absolute value of the score in the filename. 

> If score_function not None, it is possible to store its absolute value using score_name. See Notes for more details.

> If defined score_function and score_name, then the filename will be {filename_prefix}_{name}_{score_name}={abs(score)}.{ext}.

However, it seems the score (priority) is not used as an absolute value:
https://github.com/pytorch/ignite/blob/a1cae3148a92ddc82bfb1412859e04d4b51f0f25/ignite/handlers/checkpoint.py#L186

The `abs` call was removed in #679.

Discussion:
vfdev-5: Sorry, my changes broke flake8. We need split the line into two.
Joxis: @vfdev-5 I'm on it. I will also add the extra formatting.
Joxis: @vfdev-5 Would you like the decimal places formatting to be explicitly mentioned in the docs?
vfdev-5: To have 100% user-friendly documentation, yes, please, we can mention that :)
Joxis: @vfdev-5 The conflict is due to the fixed 4 decimal places, even when the original value has less decimal places. Is this desired or do you prefer rounding the value to enforce a maximum on the decimal places rather that having it always fixed to 4?
vfdev-5: @Joxis we can simply adapt true values as we expected them:
```
# assert set(os.listdir(dirname)) == set(['best_model_8_val_acc=0.61.pth', 'best_model_9_val_acc=0.7.pth'])
assert set(os.listdir(dirname)) == set(['best_model_8_val_acc=0.6100.pth', 'best_model_9_val_acc=0.7000.pth'])
```

Patch:
diff --git a/ignite/handlers/checkpoint.py b/ignite/handlers/checkpoint.py
index db893d6f..d58860f5 100644
--- a/ignite/handlers/checkpoint.py
+++ b/ignite/handlers/checkpoint.py
@@ -31,7 +31,7 @@ class Checkpoint:
         score_function (callable, optional): If not None, it should be a function taking a single argument,
             :class:`~ignite.engine.Engine` object, and returning a score (`float`). Objects with highest scores will be
             retained.
-        score_name (str, optional): If `score_function` not None, it is possible to store its absolute value using
+        score_name (str, optional): If `score_function` not None, it is possible to store its value using
             `score_name`. See Notes for more details.
         n_saved (int, optional): Number of objects that should be kept on disk. Older files will be removed. If set to
             `None`, all objects are kept.
@@ -61,11 +61,11 @@ class Checkpoint:
         The filename will be `{filename_prefix}_{name}_{global_step}_{score}.pth`.
 
         If defined `score_function` and `score_name`, then the filename will
-        be `{filename_prefix}_{name}_{score_name}={abs(score)}.{ext}`. If `global_step_transform` is provided, then
-        the filename will be `{filename_prefix}_{name}_{global_step}_{score_name}={abs(score)}.{ext}`
+        be `{filename_prefix}_{name}_{score_name}={score}.{ext}`. If `global_step_transform` is provided, then
+        the filename will be `{filename_prefix}_{name}_{global_step}_{score_name}={score}.{ext}`
 
-        For example, `score_name="val_loss"` and `score_function` that returns `-loss` (as objects with
-        highest scores will be retained), then saved filename will be `{filename_prefix}_{name}_val_loss=0.1234.pth`.
+        For example, `score_name="neg_val_loss"` and `score_function` that returns `-loss` (as objects with highest
+        scores will be retained), then saved filename will be `{filename_prefix}_{name}_neg_val_loss=-0.1234.pth`.
 
         To get the last stored filename, handler exposes attribute `last_checkpoint`:
 
@@ -183,11 +183,11 @@ class Checkpoint:
             if self._score_name is not None:
                 if len(suffix) > 0:
                     suffix += "_"
-                suffix = "{}{}={}".format(suffix, self._score_name, priority)
+                suffix = "{}{}={:.4f}".format(suffix, self._score_name, priority)
             elif self._score_function is not None:
                 if len(suffix) > 0:
                     suffix += "_"
-                suffix = "{}{}".format(suffix, priority)
+                suffix = "{}{:.4f}".format(suffix, priority)
             elif len(suffix) == 0:
                 suffix = "{}".format(priority)
 
@@ -325,7 +325,7 @@ class ModelCheckpoint(Checkpoint):
         score_function (callable, optional): if not None, it should be a function taking a single argument, an
             :class:`~ignite.engine.Engine` object, and return a score (`float`). Objects with highest scores will be
             retained.
-        score_name (str, optional): if `score_function` not None, it is possible to store its absolute value using
+        score_name (str, optional): if `score_function` not None, it is possible to store its value using
             `score_name`. See Notes for more details.
         n_saved (int, optional): Number of objects that should be kept on disk. Older files will be removed. If set to
             `None`, all objects are kept.


Test patch:
diff --git a/tests/ignite/contrib/engines/test_common.py b/tests/ignite/contrib/engines/test_common.py
index 3be8692e..55ec279c 100644
--- a/tests/ignite/contrib/engines/test_common.py
+++ b/tests/ignite/contrib/engines/test_common.py
@@ -127,7 +127,7 @@ def test_save_best_model_by_val_score(dirname, capsys):
     data = [0, ]
     trainer.run(data, max_epochs=len(acc_scores))
 
-    assert set(os.listdir(dirname)) == set(['best_model_8_val_acc=0.61.pth', 'best_model_9_val_acc=0.7.pth'])
+    assert set(os.listdir(dirname)) == set(['best_model_8_val_acc=0.6100.pth', 'best_model_9_val_acc=0.7000.pth'])
 
 
 def test_add_early_stopping_by_val_score():
diff --git a/tests/ignite/handlers/test_checkpoint.py b/tests/ignite/handlers/test_checkpoint.py
index f11e9a6f..5844bb13 100644
--- a/tests/ignite/handlers/test_checkpoint.py
+++ b/tests/ignite/handlers/test_checkpoint.py
@@ -137,7 +137,7 @@ def test_checkpoint_with_score_function():
         checkpointer(trainer)
         assert save_handler.call_count == 1
 
-        save_handler.assert_called_with(obj, "{}_0.77.pth".format(name))
+        save_handler.assert_called_with(obj, "{}_0.7700.pth".format(name))
 
         trainer.state.epoch = 12
         trainer.state.iteration = 1234
@@ -145,10 +145,10 @@ def test_checkpoint_with_score_function():
 
         checkpointer(trainer)
         assert save_handler.call_count == 2
-        save_handler.assert_called_with(obj, "{}_0.78.pth".format(name))
+        save_handler.assert_called_with(obj, "{}_0.7800.pth".format(name))
         assert save_handler.remove.call_count == 1
-        save_handler.remove.assert_called_with("{}_0.77.pth".format(name))
-        assert checkpointer.last_checkpoint == "{}_0.78.pth".format(name)
+        save_handler.remove.assert_called_with("{}_0.7700.pth".format(name))
+        assert checkpointer.last_checkpoint == "{}_0.7800.pth".format(name)
 
     model = DummyModel()
     to_save = {'model': model}
@@ -176,7 +176,7 @@ def test_checkpoint_with_score_name_and_function():
         checkpointer(trainer)
         assert save_handler.call_count == 1
 
-        save_handler.assert_called_with(obj, "{}_loss=-0.77.pth".format(name))
+        save_handler.assert_called_with(obj, "{}_loss=-0.7700.pth".format(name))
 
         trainer.state.epoch = 12
         trainer.state.iteration = 1234
@@ -184,10 +184,10 @@ def test_checkpoint_with_score_name_and_function():
 
         checkpointer(trainer)
         assert save_handler.call_count == 2
-        save_handler.assert_called_with(obj, "{}_loss=-0.76.pth".format(name))
+        save_handler.assert_called_with(obj, "{}_loss=-0.7600.pth".format(name))
         assert save_handler.remove.call_count == 1
-        save_handler.remove.assert_called_with("{}_loss=-0.77.pth".format(name))
-        assert checkpointer.last_checkpoint == "{}_loss=-0.76.pth".format(name)
+        save_handler.remove.assert_called_with("{}_loss=-0.7700.pth".format(name))
+        assert checkpointer.last_checkpoint == "{}_loss=-0.7600.pth".format(name)
 
     model = DummyModel()
     to_save = {'model': model}
@@ -218,17 +218,17 @@ def test_checkpoint_with_score_function_and_trainer_epoch():
         checkpointer(evaluator)
         assert save_handler.call_count == 1
 
-        save_handler.assert_called_with(obj, "{}_11_0.77.pth".format(name))
+        save_handler.assert_called_with(obj, "{}_11_0.7700.pth".format(name))
 
         trainer.state.epoch = 12
         evaluator.state.metrics['val_acc'] = 0.78
 
         checkpointer(evaluator)
         assert save_handler.call_count == 2
-        save_handler.assert_called_with(obj, "{}_12_0.78.pth".format(name))
+        save_handler.assert_called_with(obj, "{}_12_0.7800.pth".format(name))
         assert save_handler.remove.call_count == 1
-        save_handler.remove.assert_called_with("{}_11_0.77.pth".format(name))
-        assert checkpointer.last_checkpoint == "{}_12_0.78.pth".format(name)
+        save_handler.remove.assert_called_with("{}_11_0.7700.pth".format(name))
+        assert checkpointer.last_checkpoint == "{}_12_0.7800.pth".format(name)
 
     model = DummyModel()
     to_save = {'model': model}
@@ -256,17 +256,17 @@ def test_checkpoint_with_score_name_and_function_and_trainer_epoch():
         checkpointer(evaluator)
         assert save_handler.call_count == 1
 
-        save_handler.assert_called_with(obj, "{}_11_val_acc=0.77.pth".format(name))
+        save_handler.assert_called_with(obj, "{}_11_val_acc=0.7700.pth".format(name))
 
         trainer.state.epoch = 12
         evaluator.state.metrics['val_acc'] = 0.78
 
         checkpointer(evaluator)
         assert save_handler.call_count == 2
-        save_handler.assert_called_with(obj, "{}_12_val_acc=0.78.pth".format(name))
+        save_handler.assert_called_with(obj, "{}_12_val_acc=0.7800.pth".format(name))
         assert save_handler.remove.call_count == 1
-        save_handler.remove.assert_called_with("{}_11_val_acc=0.77.pth".format(name))
-        assert checkpointer.last_checkpoint == "{}_12_val_acc=0.78.pth".format(name)
+        save_handler.remove.assert_called_with("{}_11_val_acc=0.7700.pth".format(name))
+        assert checkpointer.last_checkpoint == "{}_12_val_acc=0.7800.pth".format(name)
 
     model = DummyModel()
     to_save = {'model': model}
@@ -440,7 +440,7 @@ def test_best_k(dirname):
     for _ in range(4):
         h(engine, to_save)
 
-    expected = ['{}_{}_{}.pth'.format(_PREFIX, 'model', i) for i in [1.2, 3.1]]
+    expected = ['{}_{}_{:.4f}.pth'.format(_PREFIX, 'model', i) for i in [1.2, 3.1]]
 
     assert sorted(os.listdir(dirname)) == expected
 
@@ -464,7 +464,7 @@ def test_best_k_with_suffix(dirname):
         engine.state.epoch += 1
         h(engine, to_save)
 
-    expected = ['{}_{}_val_loss={:.7}.pth'.format(_PREFIX, 'model', scores[e - 1]) for e in [1, 3]]
+    expected = ['{}_{}_val_loss={:.4}.pth'.format(_PREFIX, 'model', scores[e - 1]) for e in [1, 3]]
 
     assert sorted(os.listdir(dirname)) == expected